Fix eth_getCode for non-empty accounts without code (#1474)

* Fix eth_getCode for non-empty accounts without code (#1238) * Fix and debugging logs * remove debugging logs * Move to types.EmptyCodeHash * nit --------- Co-authored-by: Stefan Negovanović <[email protected]> * Lint fix * Test get code for empty code hash Foo * Cleanups * Simplify * Add comment in test * Add test for SetCode and GetCode functions --------- Co-authored-by: Trevor Porter <[email protected]>
0xPolygon · May 16, 2023 · e42d00c · e42d00c
1 parent 72fee88
commit e42d00c
Show file tree

Hide file tree

Showing 7 changed files with 82 additions and 43 deletions.
diff --git a/state/executor.go b/state/executor.go
@@ -28,8 +28,6 @@ const (
 	TxGasContractCreation uint64 = 53000 // Per transaction that creates a contract
 )
 
-var emptyCodeHashTwo = types.BytesToHash(crypto.Keccak256(nil))
-
 // GetHashByNumber returns the hash function of a block number
 type GetHashByNumber = func(i uint64) types.Hash
 
@@ -795,21 +793,14 @@ func (t *Transition) applyCall(
 	return result
 }
 
-var emptyHash types.Hash
-
 func (t *Transition) hasCodeOrNonce(addr types.Address) bool {
-	nonce := t.state.GetNonce(addr)
-	if nonce != 0 {
+	if t.state.GetNonce(addr) != 0 {
 		return true
 	}
 
 	codeHash := t.state.GetCodeHash(addr)
 
-	if codeHash != emptyCodeHashTwo && codeHash != emptyHash {
-		return true
-	}
-
-	return false
+	return codeHash != types.EmptyCodeHash && codeHash != types.ZeroHash
 }
 
 func (t *Transition) applyCreate(c *runtime.Contract, host runtime.Host) *runtime.ExecutionResult {

diff --git a/state/immutable-trie/state.go b/state/immutable-trie/state.go
@@ -47,6 +47,10 @@ func (s *State) SetCode(hash types.Hash, code []byte) {
 }
 
 func (s *State) GetCode(hash types.Hash) ([]byte, bool) {
+	if hash == types.EmptyCodeHash {
+		return []byte{}, true
+	}
+
 	return s.storage.GetCode(hash)
 }
 

diff --git a/state/immutable-trie/state_test.go b/state/immutable-trie/state_test.go
@@ -13,7 +13,6 @@ func TestState(t *testing.T) {
 func buildPreState(pre state.PreStates) state.Snapshot {
 	storage := NewMemoryStorage()
 	st := NewState(storage)
-	snap := st.NewSnapshot()
 
-	return snap
+	return st.NewSnapshot()
 }
diff --git a/state/state.go b/state/state.go
@@ -8,7 +8,6 @@ import (
 	iradix "github.com/hashicorp/go-immutable-radix"
 	"github.com/umbracle/fastrlp"
 
-	"github.com/0xPolygon/polygon-edge/crypto"
 	"github.com/0xPolygon/polygon-edge/types"
 )
 
@@ -102,8 +101,6 @@ func (a *Account) Copy() *Account {
 	return aa
 }
 
-var emptyCodeHash = crypto.Keccak256(nil)
-
 // StateObject is the internal representation of the account
 type StateObject struct {
 	Account   *Account
@@ -119,7 +116,9 @@ type StateObject struct {
 }
 
 func (s *StateObject) Empty() bool {
-	return s.Account.Nonce == 0 && s.Account.Balance.Sign() == 0 && bytes.Equal(s.Account.CodeHash, emptyCodeHash)
+	return s.Account.Nonce == 0 &&
+		s.Account.Balance.Sign() == 0 &&
+		bytes.Equal(s.Account.CodeHash, types.EmptyCodeHash.Bytes())
 }
 
 // Copy makes a copy of the state object

diff --git a/state/testing.go b/state/testing.go
@@ -9,20 +9,22 @@ import (
 	"github.com/0xPolygon/polygon-edge/types"
 )
 
-var addr1 = types.StringToAddress("1")
-var addr2 = types.StringToAddress("2")
+var (
+	addr1 = types.StringToAddress("1")
+	addr2 = types.StringToAddress("2")
 
-var hash0 = types.StringToHash("0")
-var hash1 = types.StringToHash("1")
-var hash2 = types.StringToHash("2")
+	hash0 = types.StringToHash("0")
+	hash1 = types.StringToHash("1")
+	hash2 = types.StringToHash("2")
 
-var defaultPreState = map[types.Address]*PreState{
-	addr1: {
-		State: map[types.Hash]types.Hash{
-			hash1: hash1,
+	defaultPreState = map[types.Address]*PreState{
+		addr1: {
+			State: map[types.Hash]types.Hash{
+				hash1: hash1,
+			},
 		},
-	},
-}
+	}
+)
 
 // PreState is the account prestate
 type PreState struct {
@@ -41,66 +43,76 @@ func TestState(t *testing.T, buildPreState buildPreState) {
 	t.Helper()
 	t.Parallel()
 
-	t.Run("", func(t *testing.T) {
+	t.Run("write state", func(t *testing.T) {
 		t.Parallel()
 
 		testWriteState(t, buildPreState)
 	})
-	t.Run("", func(t *testing.T) {
+	t.Run("write empty state", func(t *testing.T) {
 		t.Parallel()
 
 		testWriteEmptyState(t, buildPreState)
 	})
-	t.Run("", func(t *testing.T) {
+	t.Run("update state with empty", func(t *testing.T) {
 		t.Parallel()
 
 		testUpdateStateWithEmpty(t, buildPreState)
 	})
-	t.Run("", func(t *testing.T) {
+	t.Run("suicide account in pre-state", func(t *testing.T) {
 		t.Parallel()
 
 		testSuicideAccountInPreState(t, buildPreState)
 	})
-	t.Run("", func(t *testing.T) {
+	t.Run("suicide account", func(t *testing.T) {
 		t.Parallel()
 
 		testSuicideAccount(t, buildPreState)
 	})
-	t.Run("", func(t *testing.T) {
+	t.Run("suicide account with data", func(t *testing.T) {
 		t.Parallel()
 
 		testSuicideAccountWithData(t, buildPreState)
 	})
-	t.Run("", func(t *testing.T) {
+	t.Run("suicide coinbase", func(t *testing.T) {
 		t.Parallel()
 
 		testSuicideCoinbase(t, buildPreState)
 	})
-	t.Run("", func(t *testing.T) {
+	t.Run("suicide with intermediate commit", func(t *testing.T) {
 		t.Parallel()
 
 		testSuicideWithIntermediateCommit(t, buildPreState)
 	})
-	t.Run("", func(t *testing.T) {
+	t.Run("restart refunds", func(t *testing.T) {
 		t.Parallel()
 
 		testRestartRefunds(t, buildPreState)
 	})
-	t.Run("", func(t *testing.T) {
+	t.Run("change pre-state account balance to zero", func(t *testing.T) {
 		t.Parallel()
 
 		testChangePrestateAccountBalanceToZero(t, buildPreState)
 	})
-	t.Run("", func(t *testing.T) {
+	t.Run("change account balance to zero", func(t *testing.T) {
 		t.Parallel()
 
 		testChangeAccountBalanceToZero(t, buildPreState)
 	})
-	t.Run("", func(t *testing.T) {
+	t.Run("delete common state root", func(t *testing.T) {
 		t.Parallel()
 
 		testDeleteCommonStateRoot(t, buildPreState)
 	})
+	t.Run("get code empty code hash", func(t *testing.T) {
+		t.Parallel()
+
+		testGetCodeEmptyCodeHash(t, buildPreState)
+	})
+	t.Run("set and get code", func(t *testing.T) {
+		t.Parallel()
+
+		testSetAndGetCode(t, buildPreState)
+	})
 }
 
 func testDeleteCommonStateRoot(t *testing.T, buildPreState buildPreState) {
@@ -339,3 +351,33 @@ func testChangeAccountBalanceToZero(t *testing.T, buildPreState buildPreState) {
 	txn = newTxn(snap)
 	assert.False(t, txn.Exist(addr1))
 }
+
+func testGetCodeEmptyCodeHash(t *testing.T, buildPreState buildPreState) {
+	t.Helper()
+
+	// If empty code hash is passed, it is considered as a valid case,
+	// and in that case we are not retrieving it from the storage.
+	snap := buildPreState(nil)
+
+	code, ok := snap.GetCode(types.EmptyCodeHash)
+	assert.True(t, ok)
+	assert.Empty(t, code)
+}
+
+func testSetAndGetCode(t *testing.T, buildPreState buildPreState) {
+	t.Helper()
+
+	testCode := []byte{0x2, 0x4, 0x6, 0x8}
+	snap := buildPreState(nil)
+
+	txn := newTxn(snap)
+	txn.SetCode(addr1, testCode)
+
+	affectedObjs := txn.Commit(true)
+	snap, _ = snap.Commit(affectedObjs)
+	assert.Len(t, affectedObjs, 1)
+
+	code, ok := snap.GetCode(affectedObjs[0].CodeHash)
+	assert.True(t, ok)
+	assert.Equal(t, testCode, code)
+}
diff --git a/state/txn.go b/state/txn.go
@@ -121,7 +121,7 @@ func (txn *Txn) upsertAccount(addr types.Address, create bool, f func(object *St
 		object = &StateObject{
 			Account: &Account{
 				Balance:  big.NewInt(0),
-				CodeHash: emptyCodeHash,
+				CodeHash: types.EmptyCodeHash.Bytes(),
 				Root:     emptyStateHash,
 			},
 		}
@@ -514,7 +514,7 @@ func newStateObject(txn *Txn) *StateObject {
 	return &StateObject{
 		Account: &Account{
 			Balance:  big.NewInt(0),
-			CodeHash: emptyCodeHash,
+			CodeHash: types.EmptyCodeHash.Bytes(),
 			Root:     emptyStateHash,
 		},
 	}
@@ -524,7 +524,7 @@ func (txn *Txn) CreateAccount(addr types.Address) {
 	obj := &StateObject{
 		Account: &Account{
 			Balance:  big.NewInt(0),
-			CodeHash: emptyCodeHash,
+			CodeHash: types.EmptyCodeHash.Bytes(),
 			Root:     emptyStateHash,
 		},
 	}

diff --git a/types/types.go b/types/types.go
@@ -32,6 +32,10 @@ var (
 
 	// EmptyUncleHash is the root when there are no uncles
 	EmptyUncleHash = StringToHash("0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347")
+
+	// EmptyCodeHash is the root where there is no code.
+	// Equivalent of: `types.BytesToHash(crypto.Keccak256(nil))`
+	EmptyCodeHash = StringToHash("0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470")
 )
 
 type Hash [HashLength]byte