From 2c8ec4d1e4208f240d2f32207e5e2803ec2d816e Mon Sep 17 00:00:00 2001 From: James Date: Fri, 19 May 2023 12:48:07 -0700 Subject: [PATCH 1/8] fix: quantity-encode storage keys in eth_getProof responses --- internal/ethapi/api.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index 9a821be0c10cc..e89dbf752fee3 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -650,7 +650,7 @@ type AccountResult struct { } type StorageResult struct { - Key string `json:"key"` + Key *hexutil.Big `json:"key"` Value *hexutil.Big `json:"value"` Proof []string `json:"proof"` } @@ -680,6 +680,7 @@ func (s *BlockChainAPI) GetProof(ctx context.Context, address common.Address, st // create the proof for the storageKeys for i, hexKey := range storageKeys { key, err := decodeHash(hexKey) + keyInt := (*hexutil.Big) (new(big.Int).SetBytes(key[:])) if err != nil { return nil, err } @@ -688,9 +689,9 @@ func (s *BlockChainAPI) GetProof(ctx context.Context, address common.Address, st if storageError != nil { return nil, storageError } - storageProof[i] = StorageResult{hexKey, (*hexutil.Big)(state.GetState(address, key).Big()), toHexSlice(proof)} + storageProof[i] = StorageResult{keyInt, (*hexutil.Big)(state.GetState(address, key).Big()), toHexSlice(proof)} } else { - storageProof[i] = StorageResult{hexKey, &hexutil.Big{}, []string{}} + storageProof[i] = StorageResult{keyInt, &hexutil.Big{}, []string{}} } } From e3a3ad6eed1088ef1b84759a2ff0b575c6055bf4 Mon Sep 17 00:00:00 2001 From: James Date: Fri, 19 May 2023 14:14:14 -0700 Subject: [PATCH 2/8] lint: run golangci-lint run --fix --- internal/ethapi/api.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index e89dbf752fee3..becb4805cea71 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -680,7 +680,7 @@ func (s *BlockChainAPI) GetProof(ctx context.Context, address common.Address, st // create the proof for the storageKeys for i, hexKey := range storageKeys { key, err := decodeHash(hexKey) - keyInt := (*hexutil.Big) (new(big.Int).SetBytes(key[:])) + keyInt := (*hexutil.Big)(new(big.Int).SetBytes(key[:])) if err != nil { return nil, err } From 34b93db371000eac2e065bc1afc655241e946748 Mon Sep 17 00:00:00 2001 From: James Date: Sun, 21 May 2023 10:19:31 -0700 Subject: [PATCH 3/8] test: adjust getProof test to use quantity storage keys --- ethclient/gethclient/gethclient_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ethclient/gethclient/gethclient_test.go b/ethclient/gethclient/gethclient_test.go index 296d328484513..f8f32845fba4d 100644 --- a/ethclient/gethclient/gethclient_test.go +++ b/ethclient/gethclient/gethclient_test.go @@ -25,6 +25,7 @@ import ( "github.com/ethereum/go-ethereum" "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/common/hexutil" "github.com/ethereum/go-ethereum/consensus/ethash" "github.com/ethereum/go-ethereum/core" "github.com/ethereum/go-ethereum/core/types" @@ -230,7 +231,7 @@ func testGetProof(t *testing.T, client *rpc.Client) { if !bytes.Equal(slotValue, proof.Value.Bytes()) { t.Fatalf("invalid storage proof value, want: %v, got: %v", slotValue, proof.Value.Bytes()) } - if proof.Key != testSlot.String() { + if proof.Key != hexutil.EncodeBig(testSlot.Big()) { t.Fatalf("invalid storage proof key, want: %v, got: %v", testSlot.String(), proof.Key) } } From 17fa4b528c51c6c2c412b66f8c40d028a6bd9dc5 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Mon, 22 May 2023 10:07:00 +0200 Subject: [PATCH 4/8] ethclient/gethclient: more testing for getproof --- ethclient/gethclient/gethclient_test.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/ethclient/gethclient/gethclient_test.go b/ethclient/gethclient/gethclient_test.go index f8f32845fba4d..3f05746e62b9f 100644 --- a/ethclient/gethclient/gethclient_test.go +++ b/ethclient/gethclient/gethclient_test.go @@ -20,6 +20,7 @@ import ( "bytes" "context" "encoding/json" + "fmt" "math/big" "testing" @@ -234,6 +235,23 @@ func testGetProof(t *testing.T, client *rpc.Client) { if proof.Key != hexutil.EncodeBig(testSlot.Big()) { t.Fatalf("invalid storage proof key, want: %v, got: %v", testSlot.String(), proof.Key) } + { + // Tests with non-canon input + if result, err = ec.GetProof(context.Background(), testAddr, []string{"0x0deadbeef"}, nil); err != nil { + t.Fatal(err) + } + p, _ := new(big.Int).SetString(testSlot.String(), 0) + if have, want := result.StorageProof[0].Key, fmt.Sprintf("%#x", p); want != have { + t.Fatalf("invalid storage proof key, want: %v, have: %v", want, have) + } + if result, err = ec.GetProof(context.Background(), testAddr, []string{"0x000deadbeef"}, nil); err != nil { + t.Fatal(err) + } + p, _ = new(big.Int).SetString(testSlot.String(), 0) + if have, want := result.StorageProof[0].Key, fmt.Sprintf("%#x", p); want != have { + t.Fatalf("invalid storage proof key, want: %v, have: %v", want, have) + } + } } func testGCStats(t *testing.T, client *rpc.Client) { From 7d37b16f26584531f23e06a127225e7196ac7aa4 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Wed, 31 May 2023 09:04:22 +0200 Subject: [PATCH 5/8] internal/ethapi: go format --- internal/ethapi/api.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index 6d435b3cd91ec..71f329146af92 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -701,14 +701,14 @@ func (s *BlockChainAPI) GetProof(ctx context.Context, address common.Address, st // create the proof for the storageKeys for i, key := range keys { if storageTrie == nil { - storageProof[i] = StorageResult{(*hexutil.Big)(key.Big()), &hexutil.Big{}, []string{}} + storageProof[i] = StorageResult{(*hexutil.Big)(key.Big()), &hexutil.Big{}, []string{}} continue } var proof proofList if err := storageTrie.Prove(crypto.Keccak256(key.Bytes()), 0, &proof); err != nil { return nil, err } - storageProof[i] = StorageResult{(*hexutil.Big)(key.Big()), + storageProof[i] = StorageResult{(*hexutil.Big)(key.Big()), (*hexutil.Big)(state.GetState(address, key).Big()), proof} } From dd7158ed918b29dcc94133b4a659eaba0388b0fe Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Tue, 20 Jun 2023 13:58:33 +0200 Subject: [PATCH 6/8] internal/ethapi: apply suggestion for hash-sized storage keys --- internal/ethapi/api.go | 46 ++++++++++++++++++++++++++---------------- 1 file changed, 29 insertions(+), 17 deletions(-) diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index 71f329146af92..31ea120a7e6d3 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -650,7 +650,7 @@ type AccountResult struct { } type StorageResult struct { - Key *hexutil.Big `json:"key"` + Key string `json:"key"` Value *hexutil.Big `json:"value"` Proof []string `json:"proof"` } @@ -672,19 +672,21 @@ func (n *proofList) Delete(key []byte) error { func (s *BlockChainAPI) GetProof(ctx context.Context, address common.Address, storageKeys []string, blockNrOrHash rpc.BlockNumberOrHash) (*AccountResult, error) { var ( keys = make([]common.Hash, len(storageKeys)) + keyLengths = make([]int, len(storageKeys)) storageProof = make([]StorageResult, len(storageKeys)) storageTrie state.Trie storageHash = types.EmptyRootHash codeHash = types.EmptyCodeHash ) - // Greedily deserialize all keys. This prevents state access on invalid input + // Deserialize all keys. This prevents state access on invalid input. for i, hexKey := range storageKeys { - if key, err := decodeHash(hexKey); err != nil { + var err error + keys[i], keyLengths[i], err = decodeHash(hexKey) + if err != nil { return nil, err - } else { - keys[i] = key } } + state, _, err := s.b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash) if state == nil || err != nil { return nil, err @@ -692,28 +694,38 @@ func (s *BlockChainAPI) GetProof(ctx context.Context, address common.Address, st if storageTrie, err = state.StorageTrie(address); err != nil { return nil, err } - // if we have a storageTrie, the account exists and we must update + + // If we have a storageTrie, the account exists and we must update // the storage root hash and the code hash. if storageTrie != nil { storageHash = storageTrie.Hash() codeHash = state.GetCodeHash(address) } - // create the proof for the storageKeys + // Create the proofs for the storageKeys. for i, key := range keys { + // Output key encoding is a bit special: if the input was a 32-byte hash, it is + // returned as such. Otherwise, we apply the QUANTITY encoding mandated by the + // JSON-RPC spec for getProof. + var outputKey string + if keyLengths[i] != 32 { + outputKey = hexutil.EncodeBig(key.Big()) + } else { + outputKey = hexutil.Encode(key[:]) + } + if storageTrie == nil { - storageProof[i] = StorageResult{(*hexutil.Big)(key.Big()), &hexutil.Big{}, []string{}} + storageProof[i] = StorageResult{outputKey, &hexutil.Big{}, []string{}} continue } var proof proofList if err := storageTrie.Prove(crypto.Keccak256(key.Bytes()), 0, &proof); err != nil { return nil, err } - storageProof[i] = StorageResult{(*hexutil.Big)(key.Big()), - (*hexutil.Big)(state.GetState(address, key).Big()), - proof} + value := (*hexutil.Big)(state.GetState(address, key).Big()) + storageProof[i] = StorageResult{outputKey, value, proof} } - // create the accountProof + // Create the accountProof. accountProof, proofErr := state.GetProof(address) if proofErr != nil { return nil, proofErr @@ -732,7 +744,7 @@ func (s *BlockChainAPI) GetProof(ctx context.Context, address common.Address, st // decodeHash parses a hex-encoded 32-byte hash. The input may optionally // be prefixed by 0x and can have a byte length up to 32. -func decodeHash(s string) (common.Hash, error) { +func decodeHash(s string) (h common.Hash, inputLength int, err error) { if strings.HasPrefix(s, "0x") || strings.HasPrefix(s, "0X") { s = s[2:] } @@ -741,12 +753,12 @@ func decodeHash(s string) (common.Hash, error) { } b, err := hex.DecodeString(s) if err != nil { - return common.Hash{}, errors.New("hex string invalid") + return common.Hash{}, 0, errors.New("hex string invalid") } if len(b) > 32 { - return common.Hash{}, errors.New("hex string too long, want at most 32 bytes") + return common.Hash{}, len(b), errors.New("hex string too long, want at most 32 bytes") } - return common.BytesToHash(b), nil + return common.BytesToHash(b), len(b), nil } // GetHeaderByNumber returns the requested canonical block header. @@ -876,7 +888,7 @@ func (s *BlockChainAPI) GetStorageAt(ctx context.Context, address common.Address if state == nil || err != nil { return nil, err } - key, err := decodeHash(hexKey) + key, _, err := decodeHash(hexKey) if err != nil { return nil, fmt.Errorf("unable to decode storage key: %s", err) } From 9c5b1f0bb8124fb26b82c126c21b7fca874d7222 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Tue, 20 Jun 2023 16:25:32 +0200 Subject: [PATCH 7/8] ethclient/gethclient: update test --- ethclient/gethclient/gethclient_test.go | 55 ++++++++++++++++--------- 1 file changed, 35 insertions(+), 20 deletions(-) diff --git a/ethclient/gethclient/gethclient_test.go b/ethclient/gethclient/gethclient_test.go index 3f05746e62b9f..797e71a49852f 100644 --- a/ethclient/gethclient/gethclient_test.go +++ b/ethclient/gethclient/gethclient_test.go @@ -20,13 +20,11 @@ import ( "bytes" "context" "encoding/json" - "fmt" "math/big" "testing" "github.com/ethereum/go-ethereum" "github.com/ethereum/go-ethereum/common" - "github.com/ethereum/go-ethereum/common/hexutil" "github.com/ethereum/go-ethereum/consensus/ethash" "github.com/ethereum/go-ethereum/core" "github.com/ethereum/go-ethereum/core/types" @@ -110,6 +108,9 @@ func TestGethClient(t *testing.T) { { "TestGetProof", func(t *testing.T) { testGetProof(t, client) }, + }, { + "TestGetProofCanonicalizeKeys", + func(t *testing.T) { testGetProofCanonicalizeKeys(t, client) }, }, { "TestGCStats", func(t *testing.T) { testGCStats(t, client) }, @@ -223,6 +224,7 @@ func testGetProof(t *testing.T, client *rpc.Client) { if result.Balance.Cmp(balance) != 0 { t.Fatalf("invalid balance, want: %v got: %v", balance, result.Balance) } + // test storage if len(result.StorageProof) != 1 { t.Fatalf("invalid storage proof, want 1 proof, got %v proof(s)", len(result.StorageProof)) @@ -232,25 +234,38 @@ func testGetProof(t *testing.T, client *rpc.Client) { if !bytes.Equal(slotValue, proof.Value.Bytes()) { t.Fatalf("invalid storage proof value, want: %v, got: %v", slotValue, proof.Value.Bytes()) } - if proof.Key != hexutil.EncodeBig(testSlot.Big()) { - t.Fatalf("invalid storage proof key, want: %v, got: %v", testSlot.String(), proof.Key) + if proof.Key != testSlot.String() { + t.Fatalf("invalid storage proof key, want: %q, got: %q", testSlot.String(), proof.Key) } - { - // Tests with non-canon input - if result, err = ec.GetProof(context.Background(), testAddr, []string{"0x0deadbeef"}, nil); err != nil { - t.Fatal(err) - } - p, _ := new(big.Int).SetString(testSlot.String(), 0) - if have, want := result.StorageProof[0].Key, fmt.Sprintf("%#x", p); want != have { - t.Fatalf("invalid storage proof key, want: %v, have: %v", want, have) - } - if result, err = ec.GetProof(context.Background(), testAddr, []string{"0x000deadbeef"}, nil); err != nil { - t.Fatal(err) - } - p, _ = new(big.Int).SetString(testSlot.String(), 0) - if have, want := result.StorageProof[0].Key, fmt.Sprintf("%#x", p); want != have { - t.Fatalf("invalid storage proof key, want: %v, have: %v", want, have) - } +} + +func testGetProofCanonicalizeKeys(t *testing.T, client *rpc.Client) { + ec := New(client) + + // Tests with non-canon input for storage keys. + // Here we check that the storage key is canonicalized. + result, err := ec.GetProof(context.Background(), testAddr, []string{"0x0dEadbeef"}, nil) + if err != nil { + t.Fatal(err) + } + if result.StorageProof[0].Key != "0xdeadbeef" { + t.Fatalf("wrong storage key encoding in proof: %q", result.StorageProof[0].Key) + } + if result, err = ec.GetProof(context.Background(), testAddr, []string{"0x000deadbeef"}, nil); err != nil { + t.Fatal(err) + } + if result.StorageProof[0].Key != "0xdeadbeef" { + t.Fatalf("wrong storage key encoding in proof: %q", result.StorageProof[0].Key) + } + + // If the requested storage key is 32 bytes long, it will be returned as is. + hashSizedKey := "0x00000000000000000000000000000000000000000000000000000000deadbeef" + result, err = ec.GetProof(context.Background(), testAddr, []string{hashSizedKey}, nil) + if err != nil { + t.Fatal(err) + } + if result.StorageProof[0].Key != hashSizedKey { + t.Fatalf("wrong storage key encoding in proof: %q", result.StorageProof[0].Key) } } From b4fafd3f8752bfea00eecdab6f4f5afdd912b33a Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Tue, 20 Jun 2023 16:26:16 +0200 Subject: [PATCH 8/8] internal/ethapi: justify --- internal/ethapi/api.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index 31ea120a7e6d3..f6953482d9680 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -705,7 +705,8 @@ func (s *BlockChainAPI) GetProof(ctx context.Context, address common.Address, st for i, key := range keys { // Output key encoding is a bit special: if the input was a 32-byte hash, it is // returned as such. Otherwise, we apply the QUANTITY encoding mandated by the - // JSON-RPC spec for getProof. + // JSON-RPC spec for getProof. This behavior exists to preserve backwards + // compatibility with older client versions. var outputKey string if keyLengths[i] != 32 { outputKey = hexutil.EncodeBig(key.Big())