Merge pull request #25 from vulcanize/fixes

Misc fixes

core/tx_list.go

@@ -401,8 +401,8 @@ type priceHeap struct {
 	baseFee *big.Int
 }
 
-func (h priceHeap) Len() int      { return len(h.txs) }
-func (h priceHeap) Swap(i, j int) { h.txs[i], h.txs[j] = h.txs[j], h.txs[i] }
+func (h priceHeap) Len() int       { return len(h.txs) }
+func (h *priceHeap) Swap(i, j int) { h.txs[i], h.txs[j] = h.txs[j], h.txs[i] }
 
 func (h priceHeap) Less(i, j int) bool {
 	// Sort primarily by price, returning the cheaper one

core/types/transaction.go

@@ -438,21 +438,43 @@ func (s TxByNonce) Swap(i, j int)      { s[i], s[j] = s[j], s[i] }
 
 // TxByPrice implements both the sort and the heap interface, making it useful
 // for all at once sorting as well as individually adding and removing elements.
-type TxByPrice Transactions
+type TxByPrice struct {
+	txs     Transactions
+	baseFee *big.Int
+}
+
+func (s TxByPrice) Len() int { return len(s.txs) }
+
+// Note that this returns true if j is less than i, as the ordering needs to be from highest price to lowest
+func (s TxByPrice) Less(i, j int) bool {
+	iPrice := s.txs[i].data.Price
+	jPrice := s.txs[j].data.Price
+	if iPrice == nil {
+		iPrice = new(big.Int).Add(s.baseFee, s.txs[i].data.GasPremium)
+		if iPrice.Cmp(s.txs[i].data.FeeCap) > 0 {
+			iPrice.Set(s.txs[i].data.FeeCap)
+		}
+	}
+	if jPrice == nil {
+		jPrice = new(big.Int).Add(s.baseFee, s.txs[j].data.GasPremium)
+		if jPrice.Cmp(s.txs[j].data.FeeCap) > 0 {
+			jPrice.Set(s.txs[j].data.FeeCap)
+		}
+	}
+	return iPrice.Cmp(jPrice) > 0
+}
 
-func (s TxByPrice) Len() int           { return len(s) }
-func (s TxByPrice) Less(i, j int) bool { return s[i].data.Price.Cmp(s[j].data.Price) > 0 }
-func (s TxByPrice) Swap(i, j int)      { s[i], s[j] = s[j], s[i] }
+func (s *TxByPrice) Swap(i, j int) { s.txs[i], s.txs[j] = s.txs[j], s.txs[i] }
 
 func (s *TxByPrice) Push(x interface{}) {
-	*s = append(*s, x.(*Transaction))
+	s.txs = append(s.txs, x.(*Transaction))
 }
 
 func (s *TxByPrice) Pop() interface{} {
-	old := *s
+	old := s.txs
 	n := len(old)
 	x := old[n-1]
-	*s = old[0 : n-1]
+	s.txs = old[0 : n-1]
 	return x
 }
 
@@ -461,7 +483,7 @@ func (s *TxByPrice) Pop() interface{} {
 // entire batches of transactions for non-executable accounts.
 type TransactionsByPriceAndNonce struct {
 	txs    map[common.Address]Transactions // Per account nonce-sorted list of transactions
-	heads  TxByPrice                       // Next transaction for each unique account (price heap)
+	heads  *TxByPrice                      // Next transaction for each unique account (price heap)
 	signer Signer                          // Signer for the set of transactions
 }
 
@@ -470,19 +492,21 @@ type TransactionsByPriceAndNonce struct {
 //
 // Note, the input map is reowned so the caller should not interact any more with
 // if after providing it to the constructor.
-func NewTransactionsByPriceAndNonce(signer Signer, txs map[common.Address]Transactions) *TransactionsByPriceAndNonce {
+func NewTransactionsByPriceAndNonce(signer Signer, txs map[common.Address]Transactions, baseFee *big.Int) *TransactionsByPriceAndNonce {
 	// Initialize a price based heap with the head transactions
-	heads := make(TxByPrice, 0, len(txs))
+	heads := new(TxByPrice)
+	heads.txs = make(Transactions, 0, len(txs))
+	heads.baseFee = baseFee
 	for from, accTxs := range txs {
-		heads = append(heads, accTxs[0])
+		heads.txs = append(heads.txs, accTxs[0])
 		// Ensure the sender address is from the signer
 		acc, _ := Sender(signer, accTxs[0])
 		txs[acc] = accTxs[1:]
 		if from != acc {
 			delete(txs, from)
 		}
 	}
-	heap.Init(&heads)
+	heap.Init(heads)
 
 	// Assemble and return the transaction set
 	return &TransactionsByPriceAndNonce{
@@ -494,28 +518,28 @@ func NewTransactionsByPriceAndNonce(signer Signer, txs map[common.Address]Transa
 
 // Peek returns the next transaction by price.
 func (t *TransactionsByPriceAndNonce) Peek() *Transaction {
-	if len(t.heads) == 0 {
+	if len(t.heads.txs) == 0 {
 		return nil
 	}
-	return t.heads[0]
+	return t.heads.txs[0]
 }
 
 // Shift replaces the current best head with the next one from the same account.
 func (t *TransactionsByPriceAndNonce) Shift() {
-	acc, _ := Sender(t.signer, t.heads[0])
+	acc, _ := Sender(t.signer, t.heads.txs[0])
 	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)
+		t.heads.txs[0], t.txs[acc] = txs[0], txs[1:]
+		heap.Fix(t.heads, 0)
 	} else {
-		heap.Pop(&t.heads)
+		heap.Pop(t.heads)
 	}
 }
 
 // Pop removes the best transaction, *not* replacing it with the next one from
 // the same account. This should be used when a transaction cannot be executed
 // and hence all subsequent ones should be discarded from the same account.
 func (t *TransactionsByPriceAndNonce) Pop() {
-	heap.Pop(&t.heads)
+	heap.Pop(t.heads)
 }
 
 // Message is a fully derived transaction and implements core.Message

core/types/transaction_test.go

@@ -330,12 +330,18 @@ func TestTransactionPriceNonceSort(t *testing.T) {
 	for start, key := range keys {
 		addr := crypto.PubkeyToAddress(key.PublicKey)
 		for i := 0; i < 25; i++ {
-			tx, _ := SignTx(NewTransaction(uint64(start+i), common.Address{}, big.NewInt(100), 100, big.NewInt(int64(start+i)), nil, nil, nil), signer, key)
-			groups[addr] = append(groups[addr], tx)
+			if i%2 == 0 {
+				tx, _ := SignTx(NewTransaction(uint64(start+i), common.Address{}, big.NewInt(100), 100, big.NewInt(int64(start+i)), nil, nil, nil), signer, key)
+				groups[addr] = append(groups[addr], tx)
+			} else {
+				tx, _ := SignTx(NewTransaction(uint64(start+i), common.Address{}, big.NewInt(100), 100, nil, nil, big.NewInt(int64(start+i)), big.NewInt(int64(start+i+1))), signer, key)
+				groups[addr] = append(groups[addr], tx)
+			}
 		}
 	}
 	// Sort the transactions and cross check the nonce ordering
-	txset := NewTransactionsByPriceAndNonce(signer, groups)
+	baseFee := big.NewInt(1)
+	txset := NewTransactionsByPriceAndNonce(signer, groups, baseFee)
 
 	txs := Transactions{}
 	for tx := txset.Peek(); tx != nil; tx = txset.Peek() {
@@ -361,8 +367,22 @@ func TestTransactionPriceNonceSort(t *testing.T) {
 		if i+1 < len(txs) {
 			next := txs[i+1]
 			fromNext, _ := Sender(signer, next)
-			if fromi != fromNext && txi.GasPrice().Cmp(next.GasPrice()) < 0 {
-				t.Errorf("invalid gasprice ordering: tx #%d (A=%x P=%v) < tx #%d (A=%x P=%v)", i, fromi[:4], txi.GasPrice(), i+1, fromNext[:4], next.GasPrice())
+			iPrice := txi.GasPrice()
+			nextPrice := next.GasPrice()
+			if iPrice == nil {
+				iPrice = new(big.Int).Add(baseFee, txi.GasPremium())
+				if iPrice.Cmp(txi.FeeCap()) > 0 {
+					iPrice.Set(txi.FeeCap())
+				}
+			}
+			if nextPrice == nil {
+				nextPrice = new(big.Int).Add(baseFee, next.GasPremium())
+				if nextPrice.Cmp(next.FeeCap()) > 0 {
+					nextPrice.Set(next.FeeCap())
+				}
+			}
+			if fromi != fromNext && iPrice.Cmp(nextPrice) < 0 {
+				t.Errorf("invalid gasprice ordering: tx #%d (A=%x P=%v) < tx #%d (A=%x P=%v)", i, fromi[:4], iPrice.Uint64(), i+1, fromNext[:4], nextPrice.Uint64())
 			}
 		}
 	}
@@ -379,10 +399,9 @@ func TestTransactionJSON(t *testing.T) {
 	transactions := make([]*Transaction, 0, 50)
 	for i := uint64(0); i < 25; i++ {
 		var tx *Transaction
-		switch i % 2 {
-		case 0:
+		if i%2 == 0 {
 			tx = NewTransaction(i, common.Address{1}, common.Big0, 1, common.Big2, []byte("abcdef"), nil, nil)
-		case 1:
+		} else {
 			tx = NewContractCreation(i, common.Big0, 1, common.Big2, []byte("abcdef"), nil, nil)
 		}
 		transactions = append(transactions, tx)
@@ -427,10 +446,9 @@ func TestEIP1559TransactionJSON(t *testing.T) {
 	transactions := make([]*Transaction, 0, 50)
 	for i := uint64(0); i < 25; i++ {
 		var tx *Transaction
-		switch i % 2 {
-		case 0:
+		if i%2 == 0 {
 			tx = NewTransaction(i, common.Address{1}, common.Big0, 1, nil, []byte("abcdef"), big.NewInt(200000), big.NewInt(800000))
-		case 1:
+		} else {
 			tx = NewContractCreation(i, common.Big0, 1, nil, []byte("abcdef"), big.NewInt(200000), big.NewInt(800000))
 		}
 		transactions = append(transactions, tx)

miner/worker.go

@@ -458,11 +458,11 @@ func (w *worker) mainLoop() {
 				legacyGasPool := w.current.gasPool
 				eip1559GasPool := w.current.gp1559
 				// If EIP1559 is finalized we only accept 1559 transactions so if that pool is exhausted the block is full
-				if w.chainConfig.IsEIP1559Finalized(w.chain.CurrentBlock().Number()) && eip1559GasPool != nil && eip1559GasPool.Gas() < params.TxGas {
+				if w.chainConfig.IsEIP1559Finalized(w.current.header.Number) && eip1559GasPool != nil && eip1559GasPool.Gas() < params.TxGas {
 					continue
 				}
 				// If EIP1559 has not been initialized we only accept legacy transaction so if that pool is exhausted the block is full
-				if !w.chainConfig.IsEIP1559(w.chain.CurrentBlock().Number()) && legacyGasPool != nil && legacyGasPool.Gas() < params.TxGas {
+				if !w.chainConfig.IsEIP1559(w.current.header.Number) && legacyGasPool != nil && legacyGasPool.Gas() < params.TxGas {
 					continue
 				}
 				// When we are between EIP1559 activation and finalization we can received transactions of both types
@@ -481,7 +481,7 @@ func (w *worker) mainLoop() {
 					acc, _ := types.Sender(w.current.signer, tx)
 					txs[acc] = append(txs[acc], tx)
 				}
-				txset := types.NewTransactionsByPriceAndNonce(w.current.signer, txs)
+				txset := types.NewTransactionsByPriceAndNonce(w.current.signer, txs, w.current.header.BaseFee)
 				tcount := w.current.tcount
 				w.commitTransactions(txset, coinbase, nil)
 				// Only update the snapshot if any new transactons were added
@@ -1015,13 +1015,13 @@ func (w *worker) commitNewWork(interrupt *int32, noempty bool, timestamp int64)
 		}
 	}
 	if len(localTxs) > 0 {
-		txs := types.NewTransactionsByPriceAndNonce(w.current.signer, localTxs)
+		txs := types.NewTransactionsByPriceAndNonce(w.current.signer, localTxs, baseFee)
 		if w.commitTransactions(txs, w.coinbase, interrupt) {
 			return
 		}
 	}
 	if len(remoteTxs) > 0 {
-		txs := types.NewTransactionsByPriceAndNonce(w.current.signer, remoteTxs)
+		txs := types.NewTransactionsByPriceAndNonce(w.current.signer, remoteTxs, baseFee)
 		if w.commitTransactions(txs, w.coinbase, interrupt) {
 			return
 		}

signer/core/api.go

@@ -447,10 +447,36 @@ func logDiff(original *SignTxRequest, new *SignTxResponse) bool {
 		modified = true
 		log.Info("Gas changed by UI", "was", g0, "is", g1)
 	}
-	if g0, g1 := big.Int(original.Transaction.GasPrice), big.Int(new.Transaction.GasPrice); g0.Cmp(&g1) != 0 {
+	g0, g1 := (*big.Int)(original.Transaction.GasPrice), (*big.Int)(new.Transaction.GasPrice)
+	if g0 == nil || g1 == nil {
+		if g0 != g1 {
+			modified = true
+			log.Info("GasPrice changed by UI", "was", g0, "is", g1)
+		}
+	} else if g0.Cmp(g1) != 0 {
 		modified = true
 		log.Info("GasPrice changed by UI", "was", g0, "is", g1)
 	}
+	gp0, gp1 := (*big.Int)(original.Transaction.GasPremium), (*big.Int)(new.Transaction.GasPremium)
+	if gp0 == nil || gp1 == nil {
+		if gp0 != gp1 {
+			modified = true
+			log.Info("GasPremium changed by UI", "was", gp0, "is", gp1)
+		}
+	} else if gp0.Cmp(gp1) != 0 {
+		modified = true
+		log.Info("GasPremium changed by UI", "was", gp0, "is", gp1)
+	}
+	f0, f1 := (*big.Int)(original.Transaction.FeeCap), (*big.Int)(new.Transaction.FeeCap)
+	if f0 == nil || f1 == nil {
+		if f0 != f1 {
+			modified = true
+			log.Info("GasFee changed by UI", "was", f0, "is", f1)
+		}
+	} else if f0.Cmp(f1) != 0 {
+		modified = true
+		log.Info("GasFee changed by UI", "was", f0, "is", f1)
+	}
 	if v0, v1 := big.Int(original.Transaction.Value), big.Int(new.Transaction.Value); v0.Cmp(&v1) != 0 {
 		modified = true
 		log.Info("Value changed by UI", "was", v0, "is", v1)

signer/core/api_test.go

@@ -223,25 +223,34 @@ func TestNewAcc(t *testing.T) {
 	}
 }
 
-func mkTestTx(from common.MixedcaseAddress) core.SendTxArgs {
+func mkTestTx(from common.MixedcaseAddress, eip1559 bool) core.SendTxArgs {
 	to := common.NewMixedcaseAddress(common.HexToAddress("0x1337"))
 	gas := hexutil.Uint64(21000)
-	gasPrice := (hexutil.Big)(*big.NewInt(2000000000))
 	value := (hexutil.Big)(*big.NewInt(1e18))
 	nonce := (hexutil.Uint64)(0)
 	data := hexutil.Bytes(common.Hex2Bytes("01020304050607080a"))
 	tx := core.SendTxArgs{
-		From:     from,
-		To:       &to,
-		Gas:      gas,
-		GasPrice: gasPrice,
-		Value:    value,
-		Data:     &data,
-		Nonce:    nonce}
+		From:  from,
+		To:    &to,
+		Gas:   gas,
+		Value: value,
+		Data:  &data,
+		Nonce: nonce}
+	if eip1559 {
+		tx.GasPremium = (*hexutil.Big)(big.NewInt(1000000000))
+		tx.FeeCap = (*hexutil.Big)(big.NewInt(2000000000))
+	} else {
+		tx.GasPrice = (*hexutil.Big)(big.NewInt(2000000000))
+	}
 	return tx
 }
 
 func TestSignTx(t *testing.T) {
+	testSignTx(t, false)
+	testSignTx(t, true)
+}
+
+func testSignTx(t *testing.T, eip1559 bool) {
 	var (
 		list      []common.Address
 		res, res2 *ethapi.SignTransactionResult
@@ -258,7 +267,7 @@ func TestSignTx(t *testing.T) {
 	a := common.NewMixedcaseAddress(list[0])
 
 	methodSig := "test(uint)"
-	tx := mkTestTx(a)
+	tx := mkTestTx(a, eip1559)
 
 	control.approveCh <- "Y"
 	control.inputCh <- "wrongpassword"
@@ -321,5 +330,4 @@ func TestSignTx(t *testing.T) {
 	if bytes.Equal(res.Raw, res2.Raw) {
 		t.Error("Expected tx to be modified by UI")
 	}
-
 }

signer/core/types.go

@@ -70,12 +70,15 @@ type SendTxArgs struct {
 	From     common.MixedcaseAddress  `json:"from"`
 	To       *common.MixedcaseAddress `json:"to"`
 	Gas      hexutil.Uint64           `json:"gas"`
-	GasPrice hexutil.Big              `json:"gasPrice"`
+	GasPrice *hexutil.Big             `json:"gasPrice"`
 	Value    hexutil.Big              `json:"value"`
 	Nonce    hexutil.Uint64           `json:"nonce"`
 	// We accept "data" and "input" for backwards-compatibility reasons.
 	Data  *hexutil.Bytes `json:"data"`
 	Input *hexutil.Bytes `json:"input,omitempty"`
+	// EIP1559 fields
+	GasPremium *hexutil.Big `json:"gasPremium"`
+	FeeCap     *hexutil.Big `json:"feeCap"`
 }
 
 func (args SendTxArgs) String() string {
@@ -94,7 +97,7 @@ func (args *SendTxArgs) toTransaction() *types.Transaction {
 		input = *args.Input
 	}
 	if args.To == nil {
-		return types.NewContractCreation(uint64(args.Nonce), (*big.Int)(&args.Value), uint64(args.Gas), (*big.Int)(&args.GasPrice), input, nil, nil)
+		return types.NewContractCreation(uint64(args.Nonce), (*big.Int)(&args.Value), uint64(args.Gas), (*big.Int)(args.GasPrice), input, (*big.Int)(args.GasPremium), (*big.Int)(args.FeeCap))
 	}
-	return types.NewTransaction(uint64(args.Nonce), args.To.Address(), (*big.Int)(&args.Value), (uint64)(args.Gas), (*big.Int)(&args.GasPrice), input, nil, nil)
+	return types.NewTransaction(uint64(args.Nonce), args.To.Address(), (*big.Int)(&args.Value), (uint64)(args.Gas), (*big.Int)(args.GasPrice), input, (*big.Int)(args.GasPremium), (*big.Int)(args.FeeCap))
 }