From 3e3564bfb1555645fba5493568a93eafae2d4dac Mon Sep 17 00:00:00 2001 From: weiihann Date: Mon, 4 Nov 2024 21:45:06 +0800 Subject: [PATCH 1/4] Fixes getStorageAt undeployed contract response fix Revert "Fixes getStorageAt undeployed contract response" This reverts commit bd44ef2d79ace6abe9358c053ea3b640dc389bb0. fix fix test fix fix --- rpc/contract.go | 7 +++++++ rpc/contract_test.go | 9 +++++++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/rpc/contract.go b/rpc/contract.go index e0eb3b7c2a..a770bb284c 100644 --- a/rpc/contract.go +++ b/rpc/contract.go @@ -39,6 +39,13 @@ func (h *Handler) StorageAt(address, key felt.Felt, id BlockID) (*felt.Felt, *js } defer h.callAndLogErr(stateCloser, "Error closing state reader in getStorageAt") + // This checks if the contract exists because if a key doesn't exist in contract storage, + // the returned value is always zero and error is nil. + _, err := stateReader.ContractNonce(&address) + if err != nil { + return nil, ErrContractNotFound + } + value, err := stateReader.ContractStorage(&address, &key) if err != nil { return nil, ErrContractNotFound diff --git a/rpc/contract_test.go b/rpc/contract_test.go index 8f9e100aa3..dc84a37fd9 100644 --- a/rpc/contract_test.go +++ b/rpc/contract_test.go @@ -123,7 +123,8 @@ func TestStorageAt(t *testing.T) { t.Run("non-existent contract", func(t *testing.T) { mockReader.EXPECT().HeadState().Return(mockState, nopCloser, nil) - mockState.EXPECT().ContractStorage(gomock.Any(), gomock.Any()).Return(nil, errors.New("non-existent contract")) + // we check if the contract exists by getting its nonce + mockState.EXPECT().ContractNonce(gomock.Any()).Return(nil, errors.New("non-existent contract")) storage, rpcErr := handler.StorageAt(felt.Zero, felt.Zero, rpc.BlockID{Latest: true}) require.Nil(t, storage) @@ -132,7 +133,8 @@ func TestStorageAt(t *testing.T) { t.Run("non-existent key", func(t *testing.T) { mockReader.EXPECT().HeadState().Return(mockState, nopCloser, nil) - mockState.EXPECT().ContractStorage(gomock.Any(), gomock.Any()).Return(&felt.Zero, errors.New("non-existent key")) + // we check if the contract exists by getting its nonce + mockState.EXPECT().ContractNonce(gomock.Any()).Return(nil, errors.New("non-existent contract")) storage, rpcErr := handler.StorageAt(felt.Zero, felt.Zero, rpc.BlockID{Latest: true}) require.Nil(t, storage) @@ -143,6 +145,7 @@ func TestStorageAt(t *testing.T) { t.Run("blockID - latest", func(t *testing.T) { mockReader.EXPECT().HeadState().Return(mockState, nopCloser, nil) + mockState.EXPECT().ContractNonce(&felt.Zero).Return(nil, nil) mockState.EXPECT().ContractStorage(gomock.Any(), gomock.Any()).Return(expectedStorage, nil) storage, rpcErr := handler.StorageAt(felt.Zero, felt.Zero, rpc.BlockID{Latest: true}) @@ -152,6 +155,7 @@ func TestStorageAt(t *testing.T) { t.Run("blockID - hash", func(t *testing.T) { mockReader.EXPECT().StateAtBlockHash(&felt.Zero).Return(mockState, nopCloser, nil) + mockState.EXPECT().ContractNonce(&felt.Zero).Return(nil, nil) mockState.EXPECT().ContractStorage(gomock.Any(), gomock.Any()).Return(expectedStorage, nil) storage, rpcErr := handler.StorageAt(felt.Zero, felt.Zero, rpc.BlockID{Hash: &felt.Zero}) @@ -161,6 +165,7 @@ func TestStorageAt(t *testing.T) { t.Run("blockID - number", func(t *testing.T) { mockReader.EXPECT().StateAtBlockNumber(uint64(0)).Return(mockState, nopCloser, nil) + mockState.EXPECT().ContractNonce(&felt.Zero).Return(nil, nil) mockState.EXPECT().ContractStorage(gomock.Any(), gomock.Any()).Return(expectedStorage, nil) storage, rpcErr := handler.StorageAt(felt.Zero, felt.Zero, rpc.BlockID{Number: 0}) From 592eb95eb05fdd5ab5edce68865afb7aec12e346 Mon Sep 17 00:00:00 2001 From: weiihann Date: Tue, 5 Nov 2024 13:21:22 +0800 Subject: [PATCH 2/4] add internal error check --- rpc/contract.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/rpc/contract.go b/rpc/contract.go index a770bb284c..bee58571af 100644 --- a/rpc/contract.go +++ b/rpc/contract.go @@ -1,7 +1,10 @@ package rpc import ( + "errors" + "github.com/NethermindEth/juno/core/felt" + "github.com/NethermindEth/juno/db" "github.com/NethermindEth/juno/jsonrpc" ) @@ -43,7 +46,11 @@ func (h *Handler) StorageAt(address, key felt.Felt, id BlockID) (*felt.Felt, *js // the returned value is always zero and error is nil. _, err := stateReader.ContractNonce(&address) if err != nil { - return nil, ErrContractNotFound + if errors.Is(err, db.ErrKeyNotFound) { + return nil, ErrContractNotFound + } + h.log.Errorw("Failed to get contract nonce", "err", err) + return nil, ErrInternal } value, err := stateReader.ContractStorage(&address, &key) From ce855707d9cc5990e273d648f7778be27fb1c620 Mon Sep 17 00:00:00 2001 From: weiihann Date: Tue, 5 Nov 2024 17:45:46 +0800 Subject: [PATCH 3/4] fix test --- rpc/contract_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/rpc/contract_test.go b/rpc/contract_test.go index dc84a37fd9..4d0ee8c2c8 100644 --- a/rpc/contract_test.go +++ b/rpc/contract_test.go @@ -124,7 +124,7 @@ func TestStorageAt(t *testing.T) { t.Run("non-existent contract", func(t *testing.T) { mockReader.EXPECT().HeadState().Return(mockState, nopCloser, nil) // we check if the contract exists by getting its nonce - mockState.EXPECT().ContractNonce(gomock.Any()).Return(nil, errors.New("non-existent contract")) + mockState.EXPECT().ContractNonce(gomock.Any()).Return(nil, db.ErrKeyNotFound) storage, rpcErr := handler.StorageAt(felt.Zero, felt.Zero, rpc.BlockID{Latest: true}) require.Nil(t, storage) @@ -133,8 +133,8 @@ func TestStorageAt(t *testing.T) { t.Run("non-existent key", func(t *testing.T) { mockReader.EXPECT().HeadState().Return(mockState, nopCloser, nil) - // we check if the contract exists by getting its nonce - mockState.EXPECT().ContractNonce(gomock.Any()).Return(nil, errors.New("non-existent contract")) + mockState.EXPECT().ContractNonce(&felt.Zero).Return(nil, nil) + mockState.EXPECT().ContractStorage(gomock.Any(), gomock.Any()).Return(nil, db.ErrKeyNotFound) storage, rpcErr := handler.StorageAt(felt.Zero, felt.Zero, rpc.BlockID{Latest: true}) require.Nil(t, storage) From 9e72cf1039ec586ab9217e617f3b04daf3951181 Mon Sep 17 00:00:00 2001 From: weiihann Date: Tue, 5 Nov 2024 20:58:26 +0800 Subject: [PATCH 4/4] use class hash --- rpc/contract.go | 2 +- rpc/contract_test.go | 11 +++++------ 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/rpc/contract.go b/rpc/contract.go index bee58571af..a33f84399e 100644 --- a/rpc/contract.go +++ b/rpc/contract.go @@ -44,7 +44,7 @@ func (h *Handler) StorageAt(address, key felt.Felt, id BlockID) (*felt.Felt, *js // This checks if the contract exists because if a key doesn't exist in contract storage, // the returned value is always zero and error is nil. - _, err := stateReader.ContractNonce(&address) + _, err := stateReader.ContractClassHash(&address) if err != nil { if errors.Is(err, db.ErrKeyNotFound) { return nil, ErrContractNotFound diff --git a/rpc/contract_test.go b/rpc/contract_test.go index 4d0ee8c2c8..c9abb5214e 100644 --- a/rpc/contract_test.go +++ b/rpc/contract_test.go @@ -123,8 +123,7 @@ func TestStorageAt(t *testing.T) { t.Run("non-existent contract", func(t *testing.T) { mockReader.EXPECT().HeadState().Return(mockState, nopCloser, nil) - // we check if the contract exists by getting its nonce - mockState.EXPECT().ContractNonce(gomock.Any()).Return(nil, db.ErrKeyNotFound) + mockState.EXPECT().ContractClassHash(gomock.Any()).Return(nil, db.ErrKeyNotFound) storage, rpcErr := handler.StorageAt(felt.Zero, felt.Zero, rpc.BlockID{Latest: true}) require.Nil(t, storage) @@ -133,7 +132,7 @@ func TestStorageAt(t *testing.T) { t.Run("non-existent key", func(t *testing.T) { mockReader.EXPECT().HeadState().Return(mockState, nopCloser, nil) - mockState.EXPECT().ContractNonce(&felt.Zero).Return(nil, nil) + mockState.EXPECT().ContractClassHash(&felt.Zero).Return(nil, nil) mockState.EXPECT().ContractStorage(gomock.Any(), gomock.Any()).Return(nil, db.ErrKeyNotFound) storage, rpcErr := handler.StorageAt(felt.Zero, felt.Zero, rpc.BlockID{Latest: true}) @@ -145,7 +144,7 @@ func TestStorageAt(t *testing.T) { t.Run("blockID - latest", func(t *testing.T) { mockReader.EXPECT().HeadState().Return(mockState, nopCloser, nil) - mockState.EXPECT().ContractNonce(&felt.Zero).Return(nil, nil) + mockState.EXPECT().ContractClassHash(&felt.Zero).Return(nil, nil) mockState.EXPECT().ContractStorage(gomock.Any(), gomock.Any()).Return(expectedStorage, nil) storage, rpcErr := handler.StorageAt(felt.Zero, felt.Zero, rpc.BlockID{Latest: true}) @@ -155,7 +154,7 @@ func TestStorageAt(t *testing.T) { t.Run("blockID - hash", func(t *testing.T) { mockReader.EXPECT().StateAtBlockHash(&felt.Zero).Return(mockState, nopCloser, nil) - mockState.EXPECT().ContractNonce(&felt.Zero).Return(nil, nil) + mockState.EXPECT().ContractClassHash(&felt.Zero).Return(nil, nil) mockState.EXPECT().ContractStorage(gomock.Any(), gomock.Any()).Return(expectedStorage, nil) storage, rpcErr := handler.StorageAt(felt.Zero, felt.Zero, rpc.BlockID{Hash: &felt.Zero}) @@ -165,7 +164,7 @@ func TestStorageAt(t *testing.T) { t.Run("blockID - number", func(t *testing.T) { mockReader.EXPECT().StateAtBlockNumber(uint64(0)).Return(mockState, nopCloser, nil) - mockState.EXPECT().ContractNonce(&felt.Zero).Return(nil, nil) + mockState.EXPECT().ContractClassHash(&felt.Zero).Return(nil, nil) mockState.EXPECT().ContractStorage(gomock.Any(), gomock.Any()).Return(expectedStorage, nil) storage, rpcErr := handler.StorageAt(felt.Zero, felt.Zero, rpc.BlockID{Number: 0})