Merge pull request ethereum#14 from bas-vk/votetxpriority

core/types,core/quorum: give vote transactions priority

core/quorum/block_maker.go

@@ -60,7 +60,7 @@ func (ps *pendingState) applyTransaction(tx *types.Transaction, bc *core.BlockCh
 	return nil, logs
 }
 
-func (ps *pendingState) applyTransactions(txs *types.TransactionsByPriceAndNonce, mux *event.TypeMux, bc *core.BlockChain, cc *core.ChainConfig) (types.Transactions, types.Transactions) {
+func (ps *pendingState) applyTransactions(txs *types.TransactionsByPriorityAndNonce, mux *event.TypeMux, bc *core.BlockChain, cc *core.ChainConfig) (types.Transactions, types.Transactions) {
 	var (
 		lowGasTxs types.Transactions
 		failedTxs types.Transactions

core/quorum/block_voting.go

@@ -24,14 +24,10 @@ import (
 	"github.com/ethereum/go-ethereum/event"
 	"github.com/ethereum/go-ethereum/logger"
 	"github.com/ethereum/go-ethereum/logger/glog"
+	"github.com/ethereum/go-ethereum/params"
 	"github.com/ethereum/go-ethereum/rpc"
 )
 
-var (
-	// Block voting contract is deployed on BlockVotingContractAddress in the genesis block.
-	BlockVotingContractAddress = common.HexToAddress("0x0000000000000000000000000000000000000020")
-)
-
 const (
 	// Create bindings with: go run cmd/abigen/main.go -abi <definition> -pkg quorum -type VotingContract > core/quorum/binding.go
 	ABI = `[{"constant":false,"inputs":[{"name":"threshold","type":"uint256"}],"name":"setVoteThreshold","outputs":[],"payable":false,"type":"function"},{"constant":false,"inputs":[{"name":"addr","type":"address"}],"name":"removeBlockMaker","outputs":[],"payable":false,"type":"function"},{"constant":true,"inputs":[],"name":"voterCount","outputs":[{"name":"","type":"uint256"}],"payable":false,"type":"function"},{"constant":true,"inputs":[{"name":"","type":"address"}],"name":"canCreateBlocks","outputs":[{"name":"","type":"bool"}],"payable":false,"type":"function"},{"constant":true,"inputs":[],"name":"voteThreshold","outputs":[{"name":"","type":"uint256"}],"payable":false,"type":"function"},{"constant":true,"inputs":[{"name":"height","type":"uint256"}],"name":"getCanonHash","outputs":[{"name":"","type":"bytes32"}],"payable":false,"type":"function"},{"constant":false,"inputs":[{"name":"height","type":"uint256"},{"name":"hash","type":"bytes32"}],"name":"vote","outputs":[],"payable":false,"type":"function"},{"constant":false,"inputs":[{"name":"addr","type":"address"}],"name":"addBlockMaker","outputs":[],"payable":false,"type":"function"},{"constant":false,"inputs":[{"name":"addr","type":"address"}],"name":"removeVoter","outputs":[],"payable":false,"type":"function"},{"constant":true,"inputs":[{"name":"height","type":"uint256"},{"name":"n","type":"uint256"}],"name":"getEntry","outputs":[{"name":"","type":"bytes32"}],"payable":false,"type":"function"},{"constant":true,"inputs":[{"name":"addr","type":"address"}],"name":"isVoter","outputs":[{"name":"","type":"bool"}],"payable":false,"type":"function"},{"constant":true,"inputs":[{"name":"","type":"address"}],"name":"canVote","outputs":[{"name":"","type":"bool"}],"payable":false,"type":"function"},{"constant":true,"inputs":[],"name":"blockMakerCount","outputs":[{"name":"","type":"uint256"}],"payable":false,"type":"function"},{"constant":true,"inputs":[],"name":"getSize","outputs":[{"name":"","type":"uint256"}],"payable":false,"type":"function"},{"constant":true,"inputs":[{"name":"addr","type":"address"}],"name":"isBlockMaker","outputs":[{"name":"","type":"bool"}],"payable":false,"type":"function"},{"constant":false,"inputs":[{"name":"addr","type":"address"}],"name":"addVoter","outputs":[],"payable":false,"type":"function"},{"anonymous":false,"inputs":[{"indexed":true,"name":"sender","type":"address"},{"indexed":false,"name":"blockNumber","type":"uint256"},{"indexed":false,"name":"blockHash","type":"bytes32"}],"name":"Vote","type":"event"},{"anonymous":false,"inputs":[{"indexed":false,"name":"","type":"address"}],"name":"AddVoter","type":"event"},{"anonymous":false,"inputs":[{"indexed":false,"name":"","type":"address"}],"name":"RemovedVoter","type":"event"},{"anonymous":false,"inputs":[{"indexed":false,"name":"","type":"address"}],"name":"AddBlockMaker","type":"event"},{"anonymous":false,"inputs":[{"indexed":false,"name":"","type":"address"}],"name":"RemovedBlockMaker","type":"event"}]`
@@ -105,7 +101,7 @@ func NewBlockVoting(bc *core.BlockChain, chainConfig *core.ChainConfig, txpool *
 func (bv *BlockVoting) resetPendingState(parent *types.Block) {
 	statedb, _, err := bv.bc.StateAt(parent.Root())
 	if err != nil {
-		panic(fmt.Sprintf("State corrupt: ", err))
+		panic(fmt.Sprintf("State corrupt: %v", err))
 	}
 
 	ps := &pendingState{
@@ -118,7 +114,7 @@ func (bv *BlockVoting) resetPendingState(parent *types.Block) {
 
 	ps.gp.AddGas(ps.header.GasLimit)
 
-	txs := types.NewTransactionsByPriceAndNonce(bv.txpool.Pending())
+	txs := types.NewTransactionsByPriorityAndNonce(bv.txpool.Pending())
 
 	lowGasTxs, failedTxs := ps.applyTransactions(txs, bv.mux, bv.bc, bv.cc)
 	bv.txpool.RemoveBatch(lowGasTxs)
@@ -165,14 +161,14 @@ func (bv *BlockVoting) Start(client *rpc.Client, strat BlockMakerStrategy, voteK
 	bv.vk = voteKey
 
 	ethClient := ethclient.NewClient(client)
-	callContract, err := NewVotingContractCaller(BlockVotingContractAddress, ethClient)
+	callContract, err := NewVotingContractCaller(params.QuorumVotingContractAddr, ethClient)
 	if err != nil {
 		return err
 	}
 	bv.callContract = callContract
 
 	if voteKey != nil {
-		contract, err := NewVotingContract(BlockVotingContractAddress, ethClient)
+		contract, err := NewVotingContract(params.QuorumVotingContractAddr, ethClient)
 		if err != nil {
 			return err
 		}
@@ -290,7 +286,7 @@ func (bv *BlockVoting) run(strat BlockMakerStrategy) {
 func (bv *BlockVoting) applyTransaction(tx *types.Transaction) {
 	acc, _ := tx.From()
 	txs := map[common.Address]types.Transactions{acc: types.Transactions{tx}}
-	txset := types.NewTransactionsByPriceAndNonce(txs)
+	txset := types.NewTransactionsByPriorityAndNonce(txs)
 
 	bv.pStateMu.Lock()
 	bv.pState.applyTransactions(txset, bv.mux, bv.bc, bv.cc)
@@ -342,9 +338,6 @@ func (bv *BlockVoting) createBlock() (*types.Block, error) {
 			l.BlockHash = header.Hash()
 		}
 	}
-	//for _, log := range logs {
-	//	log.BlockHash = header.Hash()
-	//}
 
 	header.Bloom = types.CreateBloom(receipts)
 

core/quorum/block_voting_test.go

@@ -6,6 +6,7 @@ import (
 
 	"github.com/ethereum/go-ethereum/core/quorum"
 	"github.com/ethereum/go-ethereum/crypto"
+	"github.com/ethereum/go-ethereum/params"
 )
 
 var (
@@ -60,7 +61,7 @@ func genesisBlock(voteThreshold int) string {
 		addrVoteKey1.Hex(),
 		addrVoteKey2.Hex(),
 		addrBlockMaker1.Hex(),
-		quorum.BlockVotingContractAddress.Hex(),
+		params.QuorumVotingContractAddr.Hex(),
 		quorum.RuntimeCode,
 		voteThreshold,
 	)

core/types/transaction.go

@@ -28,6 +28,7 @@ import (
 
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/crypto"
+	"github.com/ethereum/go-ethereum/params"
 	"github.com/ethereum/go-ethereum/rlp"
 )
 
@@ -461,6 +462,41 @@ type TransactionsByPriceAndNonce struct {
 	heads TxByPrice                       // Next transaction for each unique account (price heap)
 }
 
+type TransactionsByPriorityAndNonce struct {
+	txs   map[common.Address]Transactions
+	heads TxByPriority
+}
+
+// TxByPriority implements both sort and the heap interface, making it useful
+// for all at once sorting as well as individual adding and removing elements.
+//
+// It will prioritise transaction to the voting contract.
+type TxByPriority Transactions
+
+func (s TxByPriority) Len() int { return len(s) }
+func (s TxByPriority) Less(i, j int) bool {
+	var (
+		iRecipient = s[i].data.Recipient
+		jRecipient = s[j].data.Recipient
+	)
+
+	// in case iReceipt is towards the voting contract and jRecipient is not towards the voting contract
+	// iReceipt is "smaller".
+	return iRecipient != nil && *iRecipient == params.QuorumVotingContractAddr && (jRecipient == nil || *jRecipient != params.QuorumVotingContractAddr)
+}
+
+func (s TxByPriority) Swap(i, j int) { s[i], s[j] = s[j], s[i] }
+func (s *TxByPriority) Push(x interface{}) {
+	*s = append(*s, x.(*Transaction))
+}
+func (s *TxByPriority) Pop() interface{} {
+	old := *s
+	n := len(old)
+	x := old[n-1]
+	*s = old[0 : n-1]
+	return x
+}
+
 // NewTransactionsByPriceAndNonce creates a transaction set that can retrieve
 // price sorted transactions in a nonce-honouring way.
 //
@@ -482,6 +518,46 @@ func NewTransactionsByPriceAndNonce(txs map[common.Address]Transactions) *Transa
 	}
 }
 
+// NewTransactionsByPriorityAndNonce creates a transaction set that can retrieve
+// vote tx sorted transactions in a nonce-honouring way.
+//
+// Note, the input map is reowned so the caller should not interact any more with
+// it after providing it to the constructor.
+func NewTransactionsByPriorityAndNonce(txs map[common.Address]Transactions) *TransactionsByPriorityAndNonce {
+	heads := make(TxByPriority, 0, len(txs))
+	for acc, accTxs := range txs {
+		heads = append(heads, accTxs[0])
+		txs[acc] = accTxs[1:]
+	}
+	heap.Init(&heads)
+
+	return &TransactionsByPriorityAndNonce{
+		txs:   txs,
+		heads: heads,
+	}
+}
+
+func (t *TransactionsByPriorityAndNonce) Peek() *Transaction {
+	if len(t.heads) == 0 {
+		return nil
+	}
+	return t.heads[0]
+}
+
+func (t *TransactionsByPriorityAndNonce) Shift() {
+	acc, _ := t.heads[0].From()
+	if txs, ok := t.txs[acc]; ok && len(txs) > 0 {
+		t.heads[0], t.txs[acc] = txs[0], txs[1:]
+		heap.Fix(&t.heads, 0)
+	} else {
+		heap.Pop(&t.heads)
+	}
+}
+
+func (t *TransactionsByPriorityAndNonce) Pop() {
+	heap.Pop(&t.heads)
+}
+
 // Peek returns the next transaction by price.
 func (t *TransactionsByPriceAndNonce) Peek() *Transaction {
 	if len(t.heads) == 0 {

core/types/transaction_test.go

@@ -20,7 +20,9 @@ import (
 	"bytes"
 	"crypto/ecdsa"
 	"math/big"
+	"math/rand"
 	"testing"
+	"time"
 
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/crypto"
@@ -118,6 +120,92 @@ func TestRecipientNormal(t *testing.T) {
 	}
 }
 
+func TestTransactionsByPriorityNonceSort(t *testing.T) {
+	votingContractAddr := common.HexToAddress("0x0000000000000000000000000000000000000020")
+
+	// Generate a batch of accounts to start with
+	keys := make([]*ecdsa.PrivateKey, 50)
+	for i := 0; i < len(keys); i++ {
+		keys[i], _ = crypto.GenerateKey()
+	}
+
+	// Generate a batch of transactions with overlapping values, but shifted nonces
+	groups := map[common.Address]Transactions{}
+
+	rand.Seed(time.Now().UnixNano())
+	for start, key := range keys {
+		addr := crypto.PubkeyToAddress(key.PublicKey)
+		for i := 0; i < 5; i++ {
+			var tx *Transaction
+			switch rand.Int() % 3 {
+			case 0:
+				tx, _ = NewTransaction(uint64(i), votingContractAddr, big.NewInt(100), big.NewInt(100), big.NewInt(int64(start+1)), nil).SignECDSA(key)
+			case 1:
+				tx, _ = NewTransaction(uint64(i), common.Address{}, big.NewInt(100), big.NewInt(100), big.NewInt(int64(start+1)), nil).SignECDSA(key)
+			default:
+				tx, _ = NewContractCreation(uint64(i), common.Big0, common.MaxBig, common.Big0, []byte{0}).SignECDSA(key)
+			}
+
+			groups[addr] = append(groups[addr], tx)
+		}
+	}
+
+	txset := NewTransactionsByPriorityAndNonce(groups)
+
+	txs := Transactions{}
+	for {
+		if tx := txset.Peek(); tx != nil {
+			txs = append(txs, tx)
+			txset.Shift()
+		} else {
+			break
+		}
+	}
+
+	for i, txi := range txs {
+		fromi, _ := txi.From()
+
+		// Make sure the nonce order is valid
+		for j, txj := range txs[i+1:] {
+			fromj, _ := txj.From()
+			if fromi == fromj && txi.Nonce() > txj.Nonce() {
+				t.Errorf("invalid nonce ordering: tx #%d (A=%x N=%v) < tx #%d (A=%x N=%v)", i, fromi[:4], txi.Nonce(), i+j, fromj[:4], txj.Nonce())
+			}
+		}
+	}
+
+	// first first non prioritised transaction
+	index := 0
+	for i, tx := range txs {
+		// search first non prioritized transaction
+		to := tx.To()
+		if to != nil && *to != votingContractAddr {
+			index = i
+			break
+		}
+	}
+
+	// ensure that all transaction after this point are non-prioritized
+	gotNonPrioritised := make(map[common.Address]bool)
+	for i, tx := range txs[index:] {
+		from, _ := tx.From()
+
+		if _, ok := gotNonPrioritised[from]; ok { // got an non-prioritised before this one which, this tx is always good
+			continue
+		} else { // didn't got a non-prioritised tx before this one, ensure this tx has no priority
+			to := tx.To()
+			if to != nil && *to == votingContractAddr {
+				for n, trans := range txs {
+					transFrom, _ := trans.From()
+					t.Logf("Tx[%d] nonce: %d, from: %x, to: %x", n, trans.Nonce(), transFrom, trans.To())
+				}
+				t.Fatalf("Found a priority tx on index %d that hasn't got no priority is should have", index+i)
+			}
+			gotNonPrioritised[from] = true
+		}
+	}
+}
+
 // Tests that transactions can be correctly sorted according to their price in
 // decreasing order, but at the same time with increasing nonces when issued by
 // the same account.
@@ -144,8 +232,9 @@ func TestTransactionPriceNonceSort(t *testing.T) {
 		if tx := txset.Peek(); tx != nil {
 			txs = append(txs, tx)
 			txset.Shift()
+		} else {
+			break
 		}
-		break
 	}
 	for i, txi := range txs {
 		fromi, _ := txi.From()