From 2a133748cb48c932dd240b5ab8eaa3e03dccf94f Mon Sep 17 00:00:00 2001 From: Bas van Kervel Date: Thu, 3 Nov 2016 15:54:33 +0100 Subject: [PATCH] core/types,core/quorum: give vote transactions priority --- core/quorum/block_maker.go | 2 +- core/quorum/block_voting.go | 19 +++---- core/quorum/block_voting_test.go | 3 +- core/types/transaction.go | 76 ++++++++++++++++++++++++++ core/types/transaction_test.go | 91 +++++++++++++++++++++++++++++++- 5 files changed, 175 insertions(+), 16 deletions(-) diff --git a/core/quorum/block_maker.go b/core/quorum/block_maker.go index ca1c5491e..a4bcb7fe4 100755 --- a/core/quorum/block_maker.go +++ b/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 diff --git a/core/quorum/block_voting.go b/core/quorum/block_voting.go index 50838241f..047f6e7d8 100755 --- a/core/quorum/block_voting.go +++ b/core/quorum/block_voting.go @@ -26,14 +26,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 -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"}]` @@ -107,7 +103,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{ @@ -120,7 +116,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) @@ -167,14 +163,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) diff --git a/core/quorum/block_voting_test.go b/core/quorum/block_voting_test.go index 552641311..221d35416 100755 --- a/core/quorum/block_voting_test.go +++ b/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, ) diff --git a/core/types/transaction.go b/core/types/transaction.go index e49dc9e88..0aacdcd20 100644 --- a/core/types/transaction.go +++ b/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 { diff --git a/core/types/transaction_test.go b/core/types/transaction_test.go index 98a78d221..24b905ba3 100644 --- a/core/types/transaction_test.go +++ b/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()