Pool updated to handle multi inputs; Handled panic where input index …

…ref non-existent block

blockchain/blockchain.go

@@ -67,13 +67,18 @@ func (bc *BlockChain) GetInputTransaction(t *TxHashPointer) *Transaction {
 }
 
 // GetAllInputs returns all the transactions referenced by a transaction
-// as inputs.
-func (bc *BlockChain) GetAllInputs(t *Transaction) []*Transaction {
+// as inputs. Returns an error if any of the transactios requested could
+// not be found.
+func (bc *BlockChain) GetAllInputs(t *Transaction) ([]*Transaction, error) {
 	txns := []*Transaction{}
 	for _, tx := range t.Inputs {
-		txns = append(txns, bc.GetInputTransaction(&tx))
+		nextTxn := bc.GetInputTransaction(&tx)
+		if nextTxn == nil {
+			return nil, errors.New("input transaction not found")
+		}
+		txns = append(txns, nextTxn)
 	}
-	return txns
+	return txns, nil
 }
 
 // ContainsTransaction returns true if the BlockChain contains the transaction

blockchain/transaction.go

@@ -131,15 +131,19 @@ func (t *Transaction) GetTotalOutputFor(recipient string) uint64 {
 
 // GetTotalInput sums the input amounts from the transaction.
 // Requires the blockchain for lookups.
-func (t *Transaction) GetTotalInput(bc *BlockChain) uint64 {
+func (t *Transaction) GetTotalInput(bc *BlockChain) (uint64, error) {
+
 	result := uint64(0)
 	// This is a bit crazy; filter all input transactions
 	// by this senders address and sum the outputs.
-	inputs := bc.GetAllInputs(t)
+	inputs, err := bc.GetAllInputs(t)
+	if err != nil {
+		return 0, err
+	}
 	for _, in := range inputs {
 		result += in.GetTotalOutputFor(t.Sender.Repr())
 	}
-	return result
+	return result, nil
 }
 
 // GetBlockRange returns the start and end block indexes for the inputs

consensus/consensus.go

@@ -22,14 +22,14 @@ func VerifyTransaction(bc *blockchain.BlockChain,
 	}
 
 	// Find the transaction input in the chain (by hash)
-	inputs := bc.GetAllInputs(t)
-	if len(inputs) == 0 {
+	inputs, err := bc.GetAllInputs(t)
+	if err != nil || len(inputs) == 0 {
 		return false, NoInputTransactions
 	}
 
 	// Check that output to sender in input is equal to outputs in t
 	out := t.GetTotalOutput()
-	in := t.GetTotalInput(bc)
+	in, err := t.GetTotalInput(bc)
 	if out != in {
 		return false, Overspend
 	}

pool/pool.go

@@ -3,8 +3,6 @@ package pool
 import (
 	"time"
 
-	log "github.com/Sirupsen/logrus"
-
 	"github.com/ubclaunchpad/cumulus/blockchain"
 	"github.com/ubclaunchpad/cumulus/common/util"
 	"github.com/ubclaunchpad/cumulus/consensus"
@@ -73,14 +71,12 @@ func getIndex(a []*PooledTransaction, target time.Time, low, high int) int {
 // Push inserts a transaction into the pool, returning
 // true if the Transaction was inserted (was valid).
 // TODO: This should return an error if could not add.
-func (p *Pool) Push(t *blockchain.Transaction, bc *blockchain.BlockChain) bool {
-	if ok, err := consensus.VerifyTransaction(bc, t); ok {
+func (p *Pool) Push(t *blockchain.Transaction, bc *blockchain.BlockChain) consensus.TransactionCode {
+	ok, code := consensus.VerifyTransaction(bc, t)
+	if ok {
 		p.set(t)
-		return true
-	} else {
-		log.Debug(err)
-		return false
 	}
+	return code
 }
 
 // PushUnsafe adds a transaction to the pool without validation.

pool/pool_test.go

@@ -5,66 +5,52 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/ubclaunchpad/cumulus/blockchain"
+	"github.com/ubclaunchpad/cumulus/consensus"
 )
 
 func TestGetAndSetTransaction(t *testing.T) {
 	p := New()
 	bc, b := blockchain.NewValidTestChainAndBlock()
-	if p.Len() != 0 {
-		t.FailNow()
-	}
-	tr := b.Transactions[1]
-	if !p.Push(tr, bc) {
-		t.FailNow()
-	}
+	assert.Equal(t, p.Size(), 0)
 
-	if p.Len() != 1 {
-		t.FailNow()
-	}
-
-	r := p.Get(tr.Input.Hash)
-	if r != tr {
-		t.FailNow()
-	}
+	tr := b.Transactions[1]
+	assert.Equal(t, p.Push(tr, bc), consensus.ValidTransaction)
+	assert.Equal(t, p.Size(), 1)
+	assert.ObjectsAreEqual(tr, p.Get(blockchain.HashSum(tr)))
 
 	p.Delete(tr)
-	if p.Len() != 0 {
-		t.FailNow()
-	}
+	assert.Equal(t, p.Size(), 0)
 }
 
 func TestSetBadTransaction(t *testing.T) {
 	p := New()
-	bc := blockchain.NewTestBlockChain()
-	if p.Push(blockchain.NewTestTransaction(), bc) {
-		t.FailNow()
-	}
+	bc, _ := blockchain.NewValidTestChainAndBlock()
+
+	// This transaction will have bad inputs.
+	txn := blockchain.NewTestTransaction()
+	code := p.Push(txn, bc)
+	assert.NotEqual(t, code, consensus.ValidTransaction)
 }
 
 func TestUpdatePool(t *testing.T) {
 	p := New()
 	bc, legitBlk := blockchain.NewValidTestChainAndBlock()
 	badBlock := blockchain.NewTestBlock()
-	if p.Update(badBlock, bc) {
-		t.FailNow()
-	}
+
+	// Make sure we cant update with a bad block.
+	assert.False(t, p.Update(badBlock, bc))
 
 	for _, tr := range legitBlk.Transactions[1:] {
 		p.Push(tr, bc)
 	}
-	if p.Len() == 0 {
-		t.FailNow()
-	}
-	if p.Len() != len(legitBlk.Transactions[1:]) {
-		t.FailNow()
-	}
 
-	if !p.Update(legitBlk, bc) {
-		t.FailNow()
-	}
-	if p.Len() != 0 {
-		t.FailNow()
-	}
+	// Assert transactions added.
+	assert.NotEqual(t, p.Size(), 0)
+	assert.Equal(t, p.Size(), len(legitBlk.Transactions[1:]))
+
+	// Assert we can add update with a legit block and it drains pool.
+	assert.True(t, p.Update(legitBlk, bc))
+	assert.Equal(t, p.Size(), 0)
 }
 
 func TestGetNewBlockEmpty(t *testing.T) {
@@ -81,13 +67,10 @@ func TestGetIndex(t *testing.T) {
 	for i := 0; i < numTxns; i++ {
 		p.PushUnsafe(blockchain.NewTestTransaction())
 	}
-	if p.GetIndex(tr) != 0 {
-		t.FailNow()
-	}
+	assert.Equal(t, p.GetIndex(tr), 0)
+
 	for i := 0; i < numTxns; i++ {
-		if p.GetIndex(p.Order[i].Transaction) != i {
-			t.FailNow()
-		}
+		assert.Equal(t, p.GetIndex(p.Order[i].Transaction), i)
 	}
 }
 
@@ -101,12 +84,13 @@ func TestNextBlock(t *testing.T) {
 		p.PushUnsafe(blockchain.NewTestTransaction())
 	}
 	b := p.NextBlock(chain, blockchain.NewWallet().Public(), 1<<18)
+
 	assert.NotNil(t, b)
 	assert.True(t, b.Len() < 1<<18)
 	assert.True(t, b.Len() > 0)
 
 	// The difference is off by one thanks to cloud transaction.
-	assert.Equal(t, len(b.Transactions), numTxns-p.Len()+1)
+	assert.Equal(t, len(b.Transactions), numTxns-p.Size()+1)
 	assert.Equal(t, blockchain.HashSum(lastBlk), b.LastBlock)
 	assert.Equal(t, uint64(0), b.Nonce)
 	assert.Equal(t, uint32(nBlks), b.BlockNumber)
@@ -125,10 +109,8 @@ func TestPop(t *testing.T) {
 func TestSetDedupes(t *testing.T) {
 	p := New()
 	t1 := blockchain.NewTestTransaction()
-	t2 := blockchain.NewTestTransaction()
-	t1.Input.Hash = t2.Input.Hash
 	p.PushUnsafe(t1)
-	p.PushUnsafe(t2)
-	assert.Equal(t, p.Peek(), t2)
-	assert.Equal(t, p.Len(), 1)
+	p.PushUnsafe(t1)
+	assert.Equal(t, p.Size(), 1)
+	assert.Equal(t, p.Peek(), t1)
 }