From a815eafbbf60018a0329393ab94ea9a8e0904784 Mon Sep 17 00:00:00 2001 From: frozen <355847+Frozen@users.noreply.github.com> Date: Sat, 11 Jan 2025 00:05:56 -0400 Subject: [PATCH 1/3] Updated the struct Message to align with Ethereum code. --- core/state_transition.go | 4 +- core/types/eth_transaction.go | 14 ++--- core/types/transaction.go | 113 +++++++++++++++++++++++----------- rpc/harmony/types.go | 2 +- 4 files changed, 86 insertions(+), 47 deletions(-) diff --git a/core/state_transition.go b/core/state_transition.go index 27c3590745..a6d29169e6 100644 --- a/core/state_transition.go +++ b/core/state_transition.go @@ -89,7 +89,7 @@ type Message interface { Value() *big.Int Nonce() uint64 - CheckNonce() bool + SkipNonceChecks() bool Data() []byte Type() types.TransactionType BlockNum() *big.Int @@ -195,7 +195,7 @@ func (st *StateTransition) buyGas() error { func (st *StateTransition) preCheck() error { // Make sure this transaction's nonce is correct. - if st.msg.CheckNonce() { + if !st.msg.SkipNonceChecks() { nonce := st.state.GetNonce(st.msg.From()) if nonce < st.msg.Nonce() { diff --git a/core/types/eth_transaction.go b/core/types/eth_transaction.go index 82e4a58805..ee8757cae7 100644 --- a/core/types/eth_transaction.go +++ b/core/types/eth_transaction.go @@ -358,13 +358,13 @@ func (tx *EthTransaction) IsEthCompatible() bool { // XXX Rename message to something less arbitrary? func (tx *EthTransaction) AsMessage(s Signer) (Message, error) { msg := Message{ - nonce: tx.data.AccountNonce, - gasLimit: tx.data.GasLimit, - gasPrice: new(big.Int).Set(tx.data.Price), - to: tx.data.Recipient, - amount: tx.data.Amount, - data: tx.data.Payload, - checkNonce: true, + nonce: tx.data.AccountNonce, + gasLimit: tx.data.GasLimit, + gasPrice: new(big.Int).Set(tx.data.Price), + to: tx.data.Recipient, + value: tx.data.Amount, + data: tx.data.Payload, + skipNonceChecks: false, } var err error diff --git a/core/types/transaction.go b/core/types/transaction.go index 590c99b8b8..a743f7d5d3 100644 --- a/core/types/transaction.go +++ b/core/types/transaction.go @@ -474,13 +474,13 @@ func (tx *Transaction) ConvertToEth() *EthTransaction { // XXX Rename message to something less arbitrary? func (tx *Transaction) AsMessage(s Signer) (Message, error) { msg := Message{ - nonce: tx.data.AccountNonce, - gasLimit: tx.data.GasLimit, - gasPrice: new(big.Int).Set(tx.data.Price), - to: tx.data.Recipient, - amount: tx.data.Amount, - data: tx.data.Payload, - checkNonce: true, + nonce: tx.data.AccountNonce, + gasLimit: tx.data.GasLimit, + gasPrice: new(big.Int).Set(tx.data.Price), + to: tx.data.Recipient, + value: tx.data.Amount, + data: tx.data.Payload, + skipNonceChecks: false, } var err error @@ -681,29 +681,68 @@ func (t *TransactionsByPriceAndNonce) Pop() { // Message is a fully derived transaction and implements core.Message // NOTE: In a future PR this will be removed. type Message struct { - to *common.Address - from common.Address - nonce uint64 - amount *big.Int - gasLimit uint64 - gasPrice *big.Int - data []byte - checkNonce bool - blockNum *big.Int - txType TransactionType -} + to *common.Address + from common.Address + nonce uint64 + value *big.Int + gasLimit uint64 + gasPrice *big.Int + gasFeeCap *big.Int + gasTipCap *big.Int + data []byte + // AccessList types.AccessList TODO: implemented in Berlin 2021-04-15 + BlobGasFeeCap *big.Int + BlobHashes []common.Hash + + // SetCodeAuthorizations []types.SetCodeAuthorization TODO: understand do we need this + + // When SkipNonceChecks is true, the message nonce is not checked against the + // account nonce in state. + // This field will be set to true for operations like RPC eth_call. + skipNonceChecks bool + + // When SkipFromEOACheck is true, the message sender is not checked to be an EOA. + skipFromEOACheck bool + + blockNum *big.Int + txType TransactionType +} + +//type Message struct { +// To *common.Address +// From common.Address +// Nonce uint64 +// Value *big.Int +// GasLimit uint64 +// GasPrice *big.Int +// GasFeeCap *big.Int +// GasTipCap *big.Int +// Data []byte +// AccessList types.AccessList +// BlobGasFeeCap *big.Int +// BlobHashes []common.Hash +// SetCodeAuthorizations []types.SetCodeAuthorization +// +// // When SkipNonceChecks is true, the message nonce is not checked against the +// // account nonce in state. +// // This field will be set to true for operations like RPC eth_call. +// SkipNonceChecks bool +// +// // When SkipFromEOACheck is true, the message sender is not checked to be an EOA. +// SkipFromEOACheck bool +//} // NewMessage returns new message. -func NewMessage(from common.Address, to *common.Address, nonce uint64, amount *big.Int, gasLimit uint64, gasPrice *big.Int, data []byte, checkNonce bool) Message { +func NewMessage(from common.Address, to *common.Address, nonce uint64, amount *big.Int, gasLimit uint64, gasPrice *big.Int, data []byte, skipNonceChecks bool) Message { return Message{ - from: from, - to: to, - nonce: nonce, - amount: amount, - gasLimit: gasLimit, - gasPrice: gasPrice, - data: data, - checkNonce: checkNonce, + from: from, + to: to, + nonce: nonce, + value: amount, + gasLimit: gasLimit, + gasPrice: gasPrice, + data: data, + skipNonceChecks: skipNonceChecks, } } @@ -711,13 +750,13 @@ func NewMessage(from common.Address, to *common.Address, nonce uint64, amount *b // always need checkNonce func NewStakingMessage(from common.Address, nonce uint64, gasLimit uint64, gasPrice *big.Int, data []byte, blockNum *big.Int) Message { return Message{ - from: from, - nonce: nonce, - gasLimit: gasLimit, - gasPrice: new(big.Int).Set(gasPrice), - data: data, - checkNonce: true, - blockNum: blockNum, + from: from, + nonce: nonce, + gasLimit: gasLimit, + gasPrice: new(big.Int).Set(gasPrice), + data: data, + skipNonceChecks: false, + blockNum: blockNum, } } @@ -738,7 +777,7 @@ func (m Message) GasPrice() *big.Int { // Value returns the value amount from Message. func (m Message) Value() *big.Int { - return m.amount + return m.value } // Gas returns gas limit of the Message. @@ -757,8 +796,8 @@ func (m Message) Data() []byte { } // CheckNonce returns checkNonce of Message. -func (m Message) CheckNonce() bool { - return m.checkNonce +func (m Message) SkipNonceChecks() bool { + return m.skipNonceChecks } // Type returns the type of message diff --git a/rpc/harmony/types.go b/rpc/harmony/types.go index e3a97afda4..479763ccee 100644 --- a/rpc/harmony/types.go +++ b/rpc/harmony/types.go @@ -72,7 +72,7 @@ func (args *CallArgs) ToMessage(globalGasCap *big.Int) types.Message { data = []byte(*args.Data) } - msg := types.NewMessage(addr, args.To, 0, value, gas, gasPrice, data, false) + msg := types.NewMessage(addr, args.To, 0, value, gas, gasPrice, data, true) return msg } From e12145a9884d97dcb90db3b455417f49bbb44ed6 Mon Sep 17 00:00:00 2001 From: Sun Hyuk Ahn Date: Thu, 13 Feb 2025 22:12:45 -0800 Subject: [PATCH 2/3] core: check if sender is eoa --- core/state_transition.go | 10 +++ core/state_transition_test.go | 155 ++++++++++++++++++++++++++++++++++ core/tx_pool.go | 3 + core/types/eth_transaction.go | 15 ++-- core/types/transaction.go | 79 +++++++---------- rpc/harmony/types.go | 2 +- 6 files changed, 209 insertions(+), 55 deletions(-) diff --git a/core/state_transition.go b/core/state_transition.go index a6d29169e6..39883bfded 100644 --- a/core/state_transition.go +++ b/core/state_transition.go @@ -22,6 +22,7 @@ import ( "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/rlp" + "github.com/harmony-one/harmony/core/state" "github.com/harmony-one/harmony/core/types" "github.com/harmony-one/harmony/core/vm" "github.com/harmony-one/harmony/internal/utils" @@ -90,6 +91,7 @@ type Message interface { Nonce() uint64 SkipNonceChecks() bool + SkipFromEOACheck() bool Data() []byte Type() types.TransactionType BlockNum() *big.Int @@ -204,6 +206,14 @@ func (st *StateTransition) preCheck() error { return ErrNonceTooLow } } + + // Ensure that the sender is an EOA (EIP-3607) + if !st.msg.SkipFromEOACheck() { + if codeHash := st.state.GetCodeHash(st.msg.From()); codeHash != state.EmptyCodeHash && codeHash != (common.Hash{}) { // getCodeHash returns EmptyCodeHash for unused accounts + return ErrSenderNotEOA + } + } + return st.buyGas() } diff --git a/core/state_transition_test.go b/core/state_transition_test.go index 7eec9d3ec1..c579dfa081 100644 --- a/core/state_transition_test.go +++ b/core/state_transition_test.go @@ -9,6 +9,7 @@ import ( "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/crypto" + "github.com/harmony-one/harmony/core/state" "github.com/harmony-one/harmony/core/types" "github.com/harmony-one/harmony/core/vm" shardingconfig "github.com/harmony-one/harmony/internal/configs/sharding" @@ -215,3 +216,157 @@ func TestCollectGasRounding(t *testing.T) { } } } + +func TestPreCheck(t *testing.T) { + key, _ := crypto.GenerateKey() + + setCode := func(db *state.DB, addr common.Address) { + code := []byte("code") + db.SetCode(addr, code, false) + } + + tests := []struct { + name string + msg types.Message + expectedError error + setup func(db *state.DB, addr common.Address) + }{ + { + name: "NonceTooHigh", + msg: types.NewMessage( + crypto.PubkeyToAddress(key.PublicKey), + nil, + 2, + big.NewInt(0), + 21000, + big.NewInt(1), + []byte{}, + false, + false, + ), + expectedError: ErrNonceTooHigh, + }, + { + name: "NonceTooLow", + msg: types.NewMessage( + crypto.PubkeyToAddress(key.PublicKey), + nil, + 0, + big.NewInt(0), + 21000, + big.NewInt(1), + []byte{}, + false, + false, + ), + expectedError: ErrNonceTooLow, + setup: func(db *state.DB, addr common.Address) { + db.SetNonce(addr, 1) + }, + }, + { + name: "SenderNotEOA", + msg: types.NewMessage( + crypto.PubkeyToAddress(key.PublicKey), + nil, + 0, + big.NewInt(0), + 21000, + big.NewInt(1), + []byte{}, + false, + false, + ), + expectedError: ErrSenderNotEOA, + setup: setCode, + }, + { + name: "SuccessfulPreCheck", + msg: types.NewMessage( + crypto.PubkeyToAddress(key.PublicKey), + nil, + 0, + big.NewInt(0), + 21000, + big.NewInt(1), + []byte{}, + false, + false, + ), + expectedError: nil, + }, + { + name: "SkipNonceChecks", + msg: types.NewMessage( + crypto.PubkeyToAddress(key.PublicKey), + nil, + 2, + big.NewInt(0), + 21000, + big.NewInt(1), + []byte{}, + true, + false, + ), + expectedError: nil, + }, + { + name: "SkipFromEOACheck", + msg: types.NewMessage( + crypto.PubkeyToAddress(key.PublicKey), + nil, + 0, + big.NewInt(0), + 21000, + big.NewInt(1), + []byte{}, + false, + true, + ), + expectedError: nil, + setup: setCode, + }, + { + name: "SkipBothChecks", + msg: types.NewMessage( + crypto.PubkeyToAddress(key.PublicKey), + nil, + 2, + big.NewInt(0), + 21000, + big.NewInt(1), + []byte{}, + true, + true, + ), + expectedError: nil, + setup: setCode, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + chain, db, header, _ := getTestEnvironment(*key) + gp := new(GasPool).AddGas(math.MaxUint64) + + initialBalance := big.NewInt(2e18) + addr := crypto.PubkeyToAddress(key.PublicKey) + db.AddBalance(addr, initialBalance) + + if tt.setup != nil { + tt.setup(db, addr) + } + + ctx := NewEVMContext(tt.msg, header, chain, nil) // coinbase is nil, no block reward + ctx.TxType = types.SameShardTx + vmenv := vm.NewEVM(ctx, db, params.TestChainConfig, vm.Config{}) + + st := NewStateTransition(vmenv, tt.msg, gp) + err := st.preCheck() + + if !errors.Is(err, tt.expectedError) { + t.Errorf("expected error %v, got %v", tt.expectedError, err) + } + }) + } +} diff --git a/core/tx_pool.go b/core/tx_pool.go index 66254a4781..2dc6eaa235 100644 --- a/core/tx_pool.go +++ b/core/tx_pool.go @@ -102,6 +102,9 @@ var ( ErrBlacklistTo = errors.New("`to` address of transaction in blacklist") ErrAllowedTxs = errors.New("transaction allowed whitelist check failed.") + + // ErrSenderNotEOA is returned if the transaction sender is a contract. + ErrSenderNotEOA = errors.New("sender not an eoa") ) var ( diff --git a/core/types/eth_transaction.go b/core/types/eth_transaction.go index ee8757cae7..ac3e0ff8a5 100644 --- a/core/types/eth_transaction.go +++ b/core/types/eth_transaction.go @@ -358,13 +358,14 @@ func (tx *EthTransaction) IsEthCompatible() bool { // XXX Rename message to something less arbitrary? func (tx *EthTransaction) AsMessage(s Signer) (Message, error) { msg := Message{ - nonce: tx.data.AccountNonce, - gasLimit: tx.data.GasLimit, - gasPrice: new(big.Int).Set(tx.data.Price), - to: tx.data.Recipient, - value: tx.data.Amount, - data: tx.data.Payload, - skipNonceChecks: false, + nonce: tx.data.AccountNonce, + gasLimit: tx.data.GasLimit, + gasPrice: new(big.Int).Set(tx.data.Price), + to: tx.data.Recipient, + value: tx.data.Amount, + data: tx.data.Payload, + skipNonceChecks: false, + skipFromEOACheck: false, } var err error diff --git a/core/types/transaction.go b/core/types/transaction.go index a743f7d5d3..1ec8ef0791 100644 --- a/core/types/transaction.go +++ b/core/types/transaction.go @@ -474,13 +474,14 @@ func (tx *Transaction) ConvertToEth() *EthTransaction { // XXX Rename message to something less arbitrary? func (tx *Transaction) AsMessage(s Signer) (Message, error) { msg := Message{ - nonce: tx.data.AccountNonce, - gasLimit: tx.data.GasLimit, - gasPrice: new(big.Int).Set(tx.data.Price), - to: tx.data.Recipient, - value: tx.data.Amount, - data: tx.data.Payload, - skipNonceChecks: false, + nonce: tx.data.AccountNonce, + gasLimit: tx.data.GasLimit, + gasPrice: new(big.Int).Set(tx.data.Price), + to: tx.data.Recipient, + value: tx.data.Amount, + data: tx.data.Payload, + skipNonceChecks: false, + skipFromEOACheck: false, } var err error @@ -708,55 +709,34 @@ type Message struct { txType TransactionType } -//type Message struct { -// To *common.Address -// From common.Address -// Nonce uint64 -// Value *big.Int -// GasLimit uint64 -// GasPrice *big.Int -// GasFeeCap *big.Int -// GasTipCap *big.Int -// Data []byte -// AccessList types.AccessList -// BlobGasFeeCap *big.Int -// BlobHashes []common.Hash -// SetCodeAuthorizations []types.SetCodeAuthorization -// -// // When SkipNonceChecks is true, the message nonce is not checked against the -// // account nonce in state. -// // This field will be set to true for operations like RPC eth_call. -// SkipNonceChecks bool -// -// // When SkipFromEOACheck is true, the message sender is not checked to be an EOA. -// SkipFromEOACheck bool -//} - // NewMessage returns new message. -func NewMessage(from common.Address, to *common.Address, nonce uint64, amount *big.Int, gasLimit uint64, gasPrice *big.Int, data []byte, skipNonceChecks bool) Message { +func NewMessage(from common.Address, to *common.Address, nonce uint64, amount *big.Int, gasLimit uint64, gasPrice *big.Int, data []byte, skipNonceChecks, skipFromEOACheck bool) Message { return Message{ - from: from, - to: to, - nonce: nonce, - value: amount, - gasLimit: gasLimit, - gasPrice: gasPrice, - data: data, - skipNonceChecks: skipNonceChecks, + from: from, + to: to, + nonce: nonce, + value: amount, + gasLimit: gasLimit, + gasPrice: gasPrice, + data: data, + skipNonceChecks: skipNonceChecks, + skipFromEOACheck: skipFromEOACheck, } } // NewStakingMessage returns new message of staking type // always need checkNonce +// TODO (sun): allow staking transactions to bypass EIP-3607 check func NewStakingMessage(from common.Address, nonce uint64, gasLimit uint64, gasPrice *big.Int, data []byte, blockNum *big.Int) Message { return Message{ - from: from, - nonce: nonce, - gasLimit: gasLimit, - gasPrice: new(big.Int).Set(gasPrice), - data: data, - skipNonceChecks: false, - blockNum: blockNum, + from: from, + nonce: nonce, + gasLimit: gasLimit, + gasPrice: new(big.Int).Set(gasPrice), + data: data, + blockNum: blockNum, + skipNonceChecks: false, + skipFromEOACheck: true, } } @@ -800,6 +780,11 @@ func (m Message) SkipNonceChecks() bool { return m.skipNonceChecks } +// SkipFromEOACheck returns skipFromEOACheck of Message. +func (m Message) SkipFromEOACheck() bool { + return m.skipFromEOACheck +} + // Type returns the type of message func (m Message) Type() TransactionType { return m.txType diff --git a/rpc/harmony/types.go b/rpc/harmony/types.go index 479763ccee..64609f98d6 100644 --- a/rpc/harmony/types.go +++ b/rpc/harmony/types.go @@ -72,7 +72,7 @@ func (args *CallArgs) ToMessage(globalGasCap *big.Int) types.Message { data = []byte(*args.Data) } - msg := types.NewMessage(addr, args.To, 0, value, gas, gasPrice, data, true) + msg := types.NewMessage(addr, args.To, 0, value, gas, gasPrice, data, true, true) return msg } From 71f2d3d89c6fe14e19ac067f3b2ee57b4459544d Mon Sep 17 00:00:00 2001 From: Sun Hyuk Ahn Date: Tue, 18 Feb 2025 10:08:04 -0800 Subject: [PATCH 3/3] tests: new staking message tests --- core/state_transition_test.go | 53 ++++++++++++++++++++++++++++++----- core/types/transaction.go | 5 ++-- 2 files changed, 48 insertions(+), 10 deletions(-) diff --git a/core/state_transition_test.go b/core/state_transition_test.go index c579dfa081..ce74aeb082 100644 --- a/core/state_transition_test.go +++ b/core/state_transition_test.go @@ -232,7 +232,7 @@ func TestPreCheck(t *testing.T) { setup func(db *state.DB, addr common.Address) }{ { - name: "NonceTooHigh", + name: "NonceTooHigh", // nonce is 0, but expected is 2 msg: types.NewMessage( crypto.PubkeyToAddress(key.PublicKey), nil, @@ -247,7 +247,7 @@ func TestPreCheck(t *testing.T) { expectedError: ErrNonceTooHigh, }, { - name: "NonceTooLow", + name: "NonceTooLow", // nonce is 1, but expected is 0 msg: types.NewMessage( crypto.PubkeyToAddress(key.PublicKey), nil, @@ -265,7 +265,7 @@ func TestPreCheck(t *testing.T) { }, }, { - name: "SenderNotEOA", + name: "SenderNotEOA", // sender has a code hash, thus not an EOA msg: types.NewMessage( crypto.PubkeyToAddress(key.PublicKey), nil, @@ -281,7 +281,7 @@ func TestPreCheck(t *testing.T) { setup: setCode, }, { - name: "SuccessfulPreCheck", + name: "SuccessfulPreCheck", // all checks pass msg: types.NewMessage( crypto.PubkeyToAddress(key.PublicKey), nil, @@ -296,7 +296,7 @@ func TestPreCheck(t *testing.T) { expectedError: nil, }, { - name: "SkipNonceChecks", + name: "SkipNonceChecks", // skip nonce checks msg: types.NewMessage( crypto.PubkeyToAddress(key.PublicKey), nil, @@ -311,7 +311,7 @@ func TestPreCheck(t *testing.T) { expectedError: nil, }, { - name: "SkipFromEOACheck", + name: "SkipFromEOACheck", // skip EOA check msg: types.NewMessage( crypto.PubkeyToAddress(key.PublicKey), nil, @@ -327,7 +327,7 @@ func TestPreCheck(t *testing.T) { setup: setCode, }, { - name: "SkipBothChecks", + name: "SkipBothChecks", // skip both checks msg: types.NewMessage( crypto.PubkeyToAddress(key.PublicKey), nil, @@ -342,6 +342,45 @@ func TestPreCheck(t *testing.T) { expectedError: nil, setup: setCode, }, + + // staking messages + { + name: "StakingWithCodeHash", // staking message with code hash + msg: types.NewStakingMessage( + crypto.PubkeyToAddress(key.PublicKey), + 0, + 21000, + big.NewInt(1), + []byte{}, + big.NewInt(1000), + ), + expectedError: nil, + setup: setCode, + }, + { + name: "StakingWithoutCodeHash", // staking message without code hash + msg: types.NewStakingMessage( + crypto.PubkeyToAddress(key.PublicKey), + 0, + 21000, + big.NewInt(1), + []byte{}, + big.NewInt(1000), + ), + expectedError: nil, + }, + { + name: "StakingNonceTooHigh", // staking message with nonce too high + msg: types.NewStakingMessage( + crypto.PubkeyToAddress(key.PublicKey), + 2, + 21000, + big.NewInt(1), + []byte{}, + big.NewInt(1000), + ), + expectedError: ErrNonceTooHigh, + }, } for _, tt := range tests { diff --git a/core/types/transaction.go b/core/types/transaction.go index 1ec8ef0791..cec5f8dfc2 100644 --- a/core/types/transaction.go +++ b/core/types/transaction.go @@ -724,9 +724,8 @@ func NewMessage(from common.Address, to *common.Address, nonce uint64, amount *b } } -// NewStakingMessage returns new message of staking type -// always need checkNonce -// TODO (sun): allow staking transactions to bypass EIP-3607 check +// NewStakingMessage returns new message of staking type. +// It requires to check the nonce and skip the EOA check. func NewStakingMessage(from common.Address, nonce uint64, gasLimit uint64, gasPrice *big.Int, data []byte, blockNum *big.Int) Message { return Message{ from: from,