diff --git a/db.go b/db.go index adfbb405..157f24ed 100644 --- a/db.go +++ b/db.go @@ -30,7 +30,7 @@ const AllShas = int64(^uint64(0) >> 1) // used to add a new backend data storage method. type Db interface { // Close cleanly shuts down the database and syncs all data. - Close() + Close() (err error) // DropAfterBlockBySha will remove any blocks from the database after // the given block. It terminates any existing transaction and performs @@ -40,7 +40,7 @@ type Db interface { // ExistsSha returns whether or not the given block hash is present in // the database. - ExistsSha(sha *btcwire.ShaHash) (exists bool) + ExistsSha(sha *btcwire.ShaHash) (exists bool, err error) // FetchBlockBySha returns a btcutil Block. The implementation may // cache the underlying data if desired. @@ -65,7 +65,7 @@ type Db interface { // ExistsTxSha returns whether or not the given tx hash is present in // the database - ExistsTxSha(sha *btcwire.ShaHash) (exists bool) + ExistsTxSha(sha *btcwire.ShaHash) (exists bool, err error) // FetchTxBySha returns some data for the given transaction hash. The // implementation may cache the underlying data if desired. @@ -75,12 +75,22 @@ type Db interface { // hashes. The implementation may cache the underlying data if desired. // This differs from FetchUnSpentTxByShaList in that it will return // the most recent known Tx, if it is fully spent or not. + // + // NOTE: This function does not return an error directly since it MUST + // return at least one TxListReply instance for each requested + // transaction. Each TxListReply instance then contains an Err field + // which can be used to detect errors. FetchTxByShaList(txShaList []*btcwire.ShaHash) []*TxListReply // FetchUnSpentTxByShaList returns a TxListReply given an array of // transaction hashes. The implementation may cache the underlying // data if desired. Fully spent transactions will not normally not // be returned in this operation. + // + // NOTE: This function does not return an error directly since it MUST + // return at least one TxListReply instance for each requested + // transaction. Each TxListReply instance then contains an Err field + // which can be used to detect errors. FetchUnSpentTxByShaList(txShaList []*btcwire.ShaHash) []*TxListReply // InsertBlock inserts raw block and transaction data from a block @@ -97,11 +107,11 @@ type Db interface { // RollbackClose discards the recent database changes to the previously // saved data at last Sync and closes the database. - RollbackClose() + RollbackClose() (err error) // Sync verifies that the database is coherent on disk and no // outstanding transactions are in flight. - Sync() + Sync() (err error) } // DriverDB defines a structure for backend drivers to use when they registered diff --git a/interface_test.go b/interface_test.go index 21500a67..31383d56 100644 --- a/interface_test.go +++ b/interface_test.go @@ -85,7 +85,13 @@ func testNewestSha(tc *testContext) bool { // testExistsSha ensures ExistsSha conforms to the interface contract. func testExistsSha(tc *testContext) bool { // The block must exist in the database. - if exists := tc.db.ExistsSha(tc.blockHash); !exists { + exists, err := tc.db.ExistsSha(tc.blockHash) + if err != nil { + tc.t.Errorf("ExistsSha (%s): block #%d (%s) unexpected error: "+ + "%v", tc.dbType, tc.blockHeight, tc.blockHash, err) + return false + } + if !exists { tc.t.Errorf("ExistsSha (%s): block #%d (%s) does not exist", tc.dbType, tc.blockHeight, tc.blockHash) return false @@ -232,7 +238,14 @@ func testExistsTxSha(tc *testContext) bool { for i, tx := range tc.block.Transactions() { // The transaction must exist in the database. txHash := tx.Sha() - if exists := tc.db.ExistsTxSha(txHash); !exists { + exists, err := tc.db.ExistsTxSha(txHash) + if err != nil { + tc.t.Errorf("ExistsTxSha (%s): block #%d (%s) tx #%d "+ + "(%s) unexpected error: %v", tc.dbType, + tc.blockHeight, tc.blockHash, i, txHash, err) + return false + } + if !exists { _, err := tc.db.FetchTxBySha(txHash) if err != nil { tc.t.Errorf("ExistsTxSha (%s): block #%d (%s) "+ diff --git a/ldb/block.go b/ldb/block.go index 25e4c1af..5e5ad56d 100644 --- a/ldb/block.go +++ b/ldb/block.go @@ -11,6 +11,7 @@ import ( "github.com/conformal/btcdb" "github.com/conformal/btcutil" "github.com/conformal/btcwire" + "github.com/conformal/goleveldb/leveldb" ) // FetchBlockBySha - return a btcutil Block @@ -186,30 +187,26 @@ func (db *LevelDb) fetchSha(sha *btcwire.ShaHash) (rbuf []byte, // ExistsSha looks up the given block hash // returns true if it is present in the database. -func (db *LevelDb) ExistsSha(sha *btcwire.ShaHash) (exists bool) { +func (db *LevelDb) ExistsSha(sha *btcwire.ShaHash) (bool, error) { db.dbLock.Lock() defer db.dbLock.Unlock() // not in cache, try database - exists = db.blkExistsSha(sha) - return + return db.blkExistsSha(sha) } // blkExistsSha looks up the given block hash // returns true if it is present in the database. // CALLED WITH LOCK HELD -func (db *LevelDb) blkExistsSha(sha *btcwire.ShaHash) bool { - +func (db *LevelDb) blkExistsSha(sha *btcwire.ShaHash) (bool, error) { _, err := db.getBlkLoc(sha) - - if err != nil { - /* - should this warn if the failure is something besides does not exist ? - log.Warnf("blkExistsSha: fail %v", err) - */ - return false + switch err { + case nil: + return true, nil + case leveldb.ErrNotFound: + return false, nil } - return true + return false, err } // FetchBlockShaByHeight returns a block hash based on its height in the diff --git a/ldb/boundary_test.go b/ldb/boundary_test.go index af49a684..f9ff23c6 100644 --- a/ldb/boundary_test.go +++ b/ldb/boundary_test.go @@ -37,14 +37,20 @@ func TestEmptyDB(t *testing.T) { } // This is a reopen test - db.Close() + if err := db.Close(); err != nil { + t.Errorf("Close: unexpected error: %v", err) + } db, err = btcdb.OpenDB("leveldb", dbname) if err != nil { t.Errorf("Failed to open test database %v", err) return } - defer db.Close() + defer func() { + if err := db.Close(); err != nil { + t.Errorf("Close: unexpected error: %v", err) + } + }() sha, height, err = db.NewestSha() if !sha.IsEqual(&btcwire.ShaHash{}) { diff --git a/ldb/dup_test.go b/ldb/dup_test.go index 8b638dfd..dca2f1fa 100644 --- a/ldb/dup_test.go +++ b/ldb/dup_test.go @@ -29,7 +29,11 @@ func Test_dupTx(t *testing.T) { } defer os.RemoveAll(dbname) defer os.RemoveAll(dbnamever) - defer db.Close() + defer func() { + if err := db.Close(); err != nil { + t.Errorf("Close: unexpected error: %v", err) + } + }() testdatafile := filepath.Join("testdata", "blocks1-256.bz2") blocks, err := loadBlocks(t, testdatafile) @@ -58,7 +62,11 @@ out: origintxsha := &txin.PreviousOutpoint.Hash txneededList = append(txneededList, origintxsha) - if !db.ExistsTxSha(origintxsha) { + exists, err := db.ExistsTxSha(origintxsha) + if err != nil { + t.Errorf("ExistsTxSha: unexpected error %v ", err) + } + if !exists { t.Errorf("referenced tx not found %v ", origintxsha) } diff --git a/ldb/insertremove_test.go b/ldb/insertremove_test.go index 27fdc486..217502ef 100644 --- a/ldb/insertremove_test.go +++ b/ldb/insertremove_test.go @@ -55,7 +55,11 @@ func testUnspentInsert(t *testing.T) { } defer os.RemoveAll(dbname) defer os.RemoveAll(dbnamever) - defer db.Close() + defer func() { + if err := db.Close(); err != nil { + t.Errorf("Close: unexpected error: %v", err) + } + }() blocks := loadblocks(t) endtest: @@ -79,10 +83,13 @@ endtest: txneededList = append(txneededList, origintxsha) txlookupList = append(txlookupList, origintxsha) - if !db.ExistsTxSha(origintxsha) { + exists, err := db.ExistsTxSha(origintxsha) + if err != nil { + t.Errorf("ExistsTxSha: unexpected error %v ", err) + } + if !exists { t.Errorf("referenced tx not found %v ", origintxsha) } - } txshaname, _ := tx.TxSha() txlookupList = append(txlookupList, &txshaname) diff --git a/ldb/leveldb.go b/ldb/leveldb.go index 04ddb474..08490df6 100644 --- a/ldb/leveldb.go +++ b/ldb/leveldb.go @@ -255,26 +255,27 @@ func CreateDB(args ...interface{}) (btcdb.Db, error) { return db, err } -func (db *LevelDb) close() { - db.lDb.Close() +func (db *LevelDb) close() error { + return db.lDb.Close() } // Sync verifies that the database is coherent on disk, // and no outstanding transactions are in flight. -func (db *LevelDb) Sync() { +func (db *LevelDb) Sync() error { db.dbLock.Lock() defer db.dbLock.Unlock() // while specified by the API, does nothing // however does grab lock to verify it does not return until other operations are complete. + return nil } // Close cleanly shuts down database, syncing all data. -func (db *LevelDb) Close() { +func (db *LevelDb) Close() error { db.dbLock.Lock() defer db.dbLock.Unlock() - db.close() + return db.close() } // DropAfterBlockBySha will remove any blocks from the database after @@ -682,9 +683,9 @@ func (db *LevelDb) processBatches() error { return nil } -func (db *LevelDb) RollbackClose() { +func (db *LevelDb) RollbackClose() error { db.dbLock.Lock() defer db.dbLock.Unlock() - db.close() + return db.close() } diff --git a/ldb/operational_test.go b/ldb/operational_test.go index cd8d6395..c0a3ca1e 100644 --- a/ldb/operational_test.go +++ b/ldb/operational_test.go @@ -44,7 +44,11 @@ func testOperationalMode(t *testing.T) { } defer os.RemoveAll(dbname) defer os.RemoveAll(dbnamever) - defer db.Close() + defer func() { + if err := db.Close(); err != nil { + t.Errorf("Close: unexpected error: %v", err) + } + }() testdatafile := filepath.Join("..", "testdata", "blocks1-256.bz2") blocks, err := loadBlocks(t, testdatafile) @@ -67,9 +71,14 @@ out: origintxsha := &txin.PreviousOutpoint.Hash txneededList = append(txneededList, origintxsha) - if !db.ExistsTxSha(origintxsha) { + exists, err := db.ExistsTxSha(origintxsha) + if err != nil { + t.Errorf("ExistsTxSha: unexpected error %v ", err) + } + if !exists { t.Errorf("referenced tx not found %v ", origintxsha) } + _, err = db.FetchTxBySha(origintxsha) if err != nil { t.Errorf("referenced tx not found %v err %v ", origintxsha, err) @@ -178,14 +187,20 @@ func testBackout(t *testing.T) { t.Errorf("Failed to open test database %v", err) return } - defer db.Close() + defer func() { + if err := db.Close(); err != nil { + t.Errorf("Close: unexpected error: %v", err) + } + }() sha, err := blocks[99].Sha() if err != nil { t.Errorf("failed to get block 99 sha err %v", err) return } - _ = db.ExistsSha(sha) + if _, err := db.ExistsSha(sha); err != nil { + t.Errorf("ExistsSha: unexpected error: %v") + } _, err = db.FetchBlockBySha(sha) if err != nil { t.Errorf("failed to load block 99 from db %v", err) @@ -197,7 +212,9 @@ func testBackout(t *testing.T) { t.Errorf("failed to get block 110 sha err %v", err) return } - _ = db.ExistsSha(sha) + if _, err := db.ExistsSha(sha); err != nil { + t.Errorf("ExistsSha: unexpected error: %v") + } _, err = db.FetchBlockBySha(sha) if err != nil { t.Errorf("loaded block 119 from db") @@ -207,7 +224,10 @@ func testBackout(t *testing.T) { block := blocks[119] mblock := block.MsgBlock() txsha, err := mblock.Transactions[0].TxSha() - exists := db.ExistsTxSha(&txsha) + exists, err := db.ExistsTxSha(&txsha) + if err != nil { + t.Errorf("ExistsTxSha: unexpected error %v ", err) + } if !exists { t.Errorf("tx %v not located db\n", txsha) } diff --git a/ldb/tx.go b/ldb/tx.go index 8f5c94c3..8201b1fb 100644 --- a/ldb/tx.go +++ b/ldb/tx.go @@ -147,7 +147,7 @@ func (db *LevelDb) formatTxFullySpent(sTxList []*spentTx) []byte { } // ExistsTxSha returns if the given tx sha exists in the database -func (db *LevelDb) ExistsTxSha(txsha *btcwire.ShaHash) (exists bool) { +func (db *LevelDb) ExistsTxSha(txsha *btcwire.ShaHash) (bool, error) { db.dbLock.Lock() defer db.dbLock.Unlock() @@ -156,15 +156,15 @@ func (db *LevelDb) ExistsTxSha(txsha *btcwire.ShaHash) (exists bool) { // existsTxSha returns if the given tx sha exists in the database.o // Must be called with the db lock held. -func (db *LevelDb) existsTxSha(txSha *btcwire.ShaHash) (exists bool) { +func (db *LevelDb) existsTxSha(txSha *btcwire.ShaHash) (bool, error) { _, _, _, _, err := db.getTxData(txSha) - if err == nil { - return true + switch err { + case nil: + return true, nil + case leveldb.ErrNotFound: + return false, nil } - - // BUG(drahn) If there was an error beside non-existant deal with it. - - return false + return false, err } // FetchTxByShaList returns the most recent tx of the name fully spent or not diff --git a/memdb/memdb.go b/memdb/memdb.go index bd2acb62..bbafd62f 100644 --- a/memdb/memdb.go +++ b/memdb/memdb.go @@ -138,14 +138,19 @@ func (db *MemDb) removeTx(msgTx *btcwire.MsgTx, txHash *btcwire.ShaHash) { // // All data is purged upon close with this implementation since it is a // memory-only database. -func (db *MemDb) Close() { +func (db *MemDb) Close() error { db.Lock() defer db.Unlock() + if db.closed { + return ErrDbClosed + } + db.blocks = nil db.blocksBySha = nil db.txns = nil db.closed = true + return nil } // DropAfterBlockBySha removes any blocks from the database after the given @@ -192,20 +197,19 @@ func (db *MemDb) DropAfterBlockBySha(sha *btcwire.ShaHash) error { // ExistsSha returns whether or not the given block hash is present in the // database. This is part of the btcdb.Db interface implementation. -func (db *MemDb) ExistsSha(sha *btcwire.ShaHash) bool { +func (db *MemDb) ExistsSha(sha *btcwire.ShaHash) (bool, error) { db.Lock() defer db.Unlock() if db.closed { - log.Warnf("ExistsSha called after db close.") - return false + return false, ErrDbClosed } if _, exists := db.blocksBySha[*sha]; exists { - return true + return true, nil } - return false + return false, nil } // FetchBlockBySha returns a btcutil.Block. The implementation may cache the @@ -346,20 +350,19 @@ func (db *MemDb) FetchHeightRange(startHeight, endHeight int64) ([]btcwire.ShaHa // ExistsTxSha returns whether or not the given transaction hash is present in // the database and is not fully spent. This is part of the btcdb.Db interface // implementation. -func (db *MemDb) ExistsTxSha(sha *btcwire.ShaHash) bool { +func (db *MemDb) ExistsTxSha(sha *btcwire.ShaHash) (bool, error) { db.Lock() defer db.Unlock() if db.closed { - log.Warnf("ExistsTxSha called after db close.") - return false + return false, ErrDbClosed } if txns, exists := db.txns[*sha]; exists { - return !isFullySpent(txns[len(txns)-1]) + return !isFullySpent(txns[len(txns)-1]), nil } - return false + return false, nil } // FetchTxBySha returns some data for the given transaction hash. The @@ -702,10 +705,10 @@ func (db *MemDb) NewestSha() (*btcwire.ShaHash, int64, error) { // The database is completely purged on close with this implementation since the // entire database is only in memory. As a result, this function behaves no // differently than Close. -func (db *MemDb) RollbackClose() { +func (db *MemDb) RollbackClose() error { // Rollback doesn't apply to a memory database, so just call Close. // Close handles the mutex locks. - db.Close() + return db.Close() } // Sync verifies that the database is coherent on disk and no outstanding @@ -714,18 +717,18 @@ func (db *MemDb) RollbackClose() { // // This implementation does not write any data to disk, so this function only // grabs a lock to ensure it doesn't return until other operations are complete. -func (db *MemDb) Sync() { +func (db *MemDb) Sync() error { db.Lock() defer db.Unlock() if db.closed { - log.Warnf("Sync called after db close.") + return ErrDbClosed } // There is nothing extra to do to sync the memory database. However, // the lock is still grabbed to ensure the function does not return // until other operations are complete. - return + return nil } // newMemDb returns a new memory-only database ready for block inserts. diff --git a/memdb/memdb_test.go b/memdb/memdb_test.go index 8e33de47..ddbe3717 100644 --- a/memdb/memdb_test.go +++ b/memdb/memdb_test.go @@ -29,15 +29,17 @@ func TestClosed(t *testing.T) { if err != nil { t.Errorf("InsertBlock: %v", err) } - db.Close() + if err := db.Close(); err != nil { + t.Errorf("Close: unexpected error %v", err) + } genesisHash := btcnet.MainNetParams.GenesisHash if err := db.DropAfterBlockBySha(genesisHash); err != memdb.ErrDbClosed { t.Errorf("DropAfterBlockBySha: unexpected error %v", err) } - if exists := db.ExistsSha(genesisHash); exists != false { - t.Errorf("ExistsSha: genesis hash exists after close") + if _, err := db.ExistsSha(genesisHash); err != memdb.ErrDbClosed { + t.Errorf("ExistsSha: Unexpected error: %v", err) } if _, err := db.FetchBlockBySha(genesisHash); err != memdb.ErrDbClosed { @@ -57,9 +59,8 @@ func TestClosed(t *testing.T) { if err != nil { t.Errorf("TxSha: unexpected error %v", err) } - if exists := db.ExistsTxSha(&coinbaseHash); exists != false { - t.Errorf("ExistsTxSha: hash %v exists when it shouldn't", - &coinbaseHash) + if _, err := db.ExistsTxSha(&coinbaseHash); err != memdb.ErrDbClosed { + t.Errorf("ExistsTxSha: unexpected error %v", err) } if _, err := db.FetchTxBySha(genesisHash); err != memdb.ErrDbClosed { @@ -103,9 +104,15 @@ func TestClosed(t *testing.T) { t.Errorf("NewestSha: unexpected error %v", err) } - // The following calls don't return errors from the interface to be able - // to detect a closed database, so just call them to ensure there are no - // panics. - db.Sync() - db.RollbackClose() + if err := db.Sync(); err != memdb.ErrDbClosed { + t.Errorf("Sync: unexpected error %v", err) + } + + if err := db.RollbackClose(); err != memdb.ErrDbClosed { + t.Errorf("RollbackClose: unexpected error %v", err) + } + + if err := db.Close(); err != memdb.ErrDbClosed { + t.Errorf("Close: unexpected error %v", err) + } }