From 909c615286605389b948db0a9cff0c7c46ecd6fe Mon Sep 17 00:00:00 2001
From: Steven Allen <steven@stebalien.com>
Date: Wed, 15 Feb 2023 13:39:02 -0800
Subject: [PATCH] fix: eth: correctly decode EthGetStorageAt output

We cbor-encode it. Also:

1. Actually use the passed block param.
2. Check if the target actor is an EVM actor to avoid nonsense outputs.

fixes https://github.com/filecoin-project/ref-fvm/issues/1621
---
 itests/fevm_test.go   | 14 ++++++++++++++
 node/impl/full/eth.go | 39 ++++++++++++++++++++++++++++++++++-----
 2 files changed, 48 insertions(+), 5 deletions(-)

diff --git a/itests/fevm_test.go b/itests/fevm_test.go
index 3018bf63ddd..b4d9f04961b 100644
--- a/itests/fevm_test.go
+++ b/itests/fevm_test.go
@@ -265,6 +265,20 @@ func TestFEVMDelegateCall(t *testing.T) {
 	expectedResultActor, err := hex.DecodeString("0000000000000000000000000000000000000000000000000000000000000000")
 	require.NoError(t, err)
 	require.Equal(t, result, expectedResultActor)
+
+	// The implementation's storage should not have been updated.
+	actorAddrEth, err := ethtypes.EthAddressFromFilecoinAddress(actorAddr)
+	require.NoError(t, err)
+	value, err := client.EVM().EthGetStorageAt(ctx, actorAddrEth, nil, "latest")
+	require.NoError(t, err)
+	require.Equal(t, make([]byte, 32), value)
+
+	// The storage actor's storage _should_ have been updated
+	storageAddrEth, err := ethtypes.EthAddressFromFilecoinAddress(storageAddr)
+	require.NoError(t, err)
+	value, err = client.EVM().EthGetStorageAt(ctx, storageAddrEth, nil, "latest")
+	require.NoError(t, err)
+	require.Equal(t, expectedResult, value)
 }
 
 // TestFEVMDelegateCallRevert makes a delegatecall action and then calls revert.
diff --git a/node/impl/full/eth.go b/node/impl/full/eth.go
index d41a15c883c..599575957f8 100644
--- a/node/impl/full/eth.go
+++ b/node/impl/full/eth.go
@@ -506,13 +506,18 @@ func (a *EthModule) EthGetCode(ctx context.Context, ethAddr ethtypes.EthAddress,
 }
 
 func (a *EthModule) EthGetStorageAt(ctx context.Context, ethAddr ethtypes.EthAddress, position ethtypes.EthBytes, blkParam string) (ethtypes.EthBytes, error) {
+	ts, err := a.parseBlkParam(ctx, blkParam)
+	if err != nil {
+		return nil, xerrors.Errorf("cannot parse block param: %s", blkParam)
+	}
+
 	l := len(position)
 	if l > 32 {
 		return nil, fmt.Errorf("supplied storage key is too long")
 	}
 
 	// pad with zero bytes if smaller than 32 bytes
-	position = append(make([]byte, 32-l, 32-l), position...)
+	position = append(make([]byte, 32-l), position...)
 
 	to, err := ethAddr.ToFilecoinAddress()
 	if err != nil {
@@ -525,6 +530,20 @@ func (a *EthModule) EthGetStorageAt(ctx context.Context, ethAddr ethtypes.EthAdd
 		return nil, fmt.Errorf("failed to construct system sender address: %w", err)
 	}
 
+	actor, err := a.StateManager.LoadActor(ctx, to, ts)
+	if err != nil {
+		if xerrors.Is(err, types.ErrActorNotFound) {
+			return nil, nil
+		}
+		return nil, xerrors.Errorf("failed to lookup contract %s: %w", ethAddr, err)
+	}
+
+	// Not a contract. Technically, we should return an empty slot... but an error is more user
+	// friendly, IMO.
+	if !builtinactors.IsEvmActor(actor.Code) {
+		return nil, xerrors.Errorf("actor is not an ethereum contract")
+	}
+
 	params, err := actors.SerializeParams(&evm.GetStorageAtParams{
 		StorageKey: *(*[32]byte)(position),
 	})
@@ -543,8 +562,6 @@ func (a *EthModule) EthGetStorageAt(ctx context.Context, ethAddr ethtypes.EthAdd
 		GasPremium: big.Zero(),
 	}
 
-	ts := a.Chain.GetHeaviestTipSet()
-
 	// Try calling until we find a height with no migration.
 	var res *api.InvocResult
 	for {
@@ -563,10 +580,22 @@ func (a *EthModule) EthGetStorageAt(ctx context.Context, ethAddr ethtypes.EthAdd
 	}
 
 	if res.MsgRct == nil {
-		return nil, fmt.Errorf("no message receipt")
+		return nil, xerrors.Errorf("no message receipt")
 	}
 
-	return res.MsgRct.Return, nil
+	if res.MsgRct.ExitCode.IsError() {
+		return nil, xerrors.Errorf("failed to lookup storage slot: %s", res.Error)
+	}
+
+	var ret abi.CborBytes
+	if err := ret.UnmarshalCBOR(bytes.NewReader(res.MsgRct.Return)); err != nil {
+		return nil, xerrors.Errorf("failed to unmarshal storage slot: %w", err)
+	}
+
+	// pad with zero bytes if smaller than 32 bytes
+	ret = append(make([]byte, 32-l), ret...)
+
+	return ethtypes.EthBytes(ret), nil
 }
 
 func (a *EthModule) EthGetBalance(ctx context.Context, address ethtypes.EthAddress, blkParam string) (ethtypes.EthBigInt, error) {