Add errors to all interface methods.

This commit adds error returns to all of the Db interface methods except
for FetchTxByShaList and FetchUnSpentTxByShaList since they expose the
errors on each individual transaction.

It also updates all tests and code for both the leveldb and memdb drivers
for the changes.

Closes #5.

ok @drahn

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

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) "+

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

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{}) {

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)
 				}
 

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)

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()
 }

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)
 	}

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

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.

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)
+	}
 }