Skip to content

Commit

Permalink
fix: eth: correctly decode EthGetStorageAt output (#10284)
Browse files Browse the repository at this point in the history
* 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 filecoin-project/ref-fvm#1621
  • Loading branch information
Stebalien authored Feb 17, 2023
1 parent a2b996e commit 5854d72
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 5 deletions.
14 changes: 14 additions & 0 deletions itests/fevm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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, ethtypes.EthBytes(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, ethtypes.EthBytes(expectedResult), value)
}

// TestFEVMDelegateCallRevert makes a delegatecall action and then calls revert.
Expand Down
37 changes: 32 additions & 5 deletions node/impl/full/eth.go
Original file line number Diff line number Diff line change
Expand Up @@ -510,13 +510,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, 32), position...)

to, err := ethAddr.ToFilecoinAddress()
if err != nil {
Expand All @@ -529,6 +534,18 @@ 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 ethtypes.EthBytes(make([]byte, 32)), nil
}
return nil, xerrors.Errorf("failed to lookup contract %s: %w", ethAddr, err)
}

if !builtinactors.IsEvmActor(actor.Code) {
return ethtypes.EthBytes(make([]byte, 32)), nil
}

params, err := actors.SerializeParams(&evm.GetStorageAtParams{
StorageKey: *(*[32]byte)(position),
})
Expand All @@ -547,8 +564,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 {
Expand All @@ -567,10 +582,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")
}

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)
}

return res.MsgRct.Return, nil
// pad with zero bytes if smaller than 32 bytes
ret = append(make([]byte, 32-len(ret), 32), ret...)

return ethtypes.EthBytes(ret), nil
}

func (a *EthModule) EthGetBalance(ctx context.Context, address ethtypes.EthAddress, blkParam string) (ethtypes.EthBigInt, error) {
Expand Down

0 comments on commit 5854d72

Please sign in to comment.