Skip to content

Commit f3d9fdd

Browse files
authored
fix getStorage and setStorage RPCs (#385)
- getStorage should return empty data for an inexistent account (we were previously returning error) - setStorage should also work for inexistent accounts or EOAs
1 parent 1d6e7fa commit f3d9fdd

File tree

5 files changed

+149
-58
lines changed

5 files changed

+149
-58
lines changed

.github/workflows/test.yml

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -53,19 +53,19 @@ jobs:
5353
force_orphan: true
5454

5555
doctest:
56-
runs-on: ubuntu-latest
56+
runs-on: parity-large-new
5757
timeout-minutes: 60
5858
steps:
5959
- uses: actions/checkout@v4
6060
- uses: dtolnay/[email protected]
61+
- name: Install build tools
62+
run: |
63+
sudo apt-get update
64+
sudo apt-get install -y clang libclang-dev unzip build-essential
6165
- name: Install protobuf-compiler
6266
uses: arduino/setup-protoc@v3
6367
with:
6468
repo-token: ${{ secrets.GITHUB_TOKEN }}
65-
- name: Install clang on ubuntu
66-
run: |
67-
sudo apt-get update
68-
sudo apt-get install -y clang libclang-dev
6969
- uses: dtolnay/[email protected]
7070
with:
7171
target: wasm32-unknown-unknown

crates/anvil-polkadot/src/api_server/server.rs

Lines changed: 60 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -564,13 +564,21 @@ impl ApiServer {
564564
addr: Address,
565565
slot: U256,
566566
block: Option<BlockId>,
567-
) -> Result<Bytes> {
567+
) -> Result<B256> {
568568
node_info!("eth_getStorageAt");
569569
let hash = self.get_block_hash_for_tag(block).await?;
570570
let runtime_api = self.eth_rpc_client.runtime_api(hash);
571-
let bytes =
572-
runtime_api.get_storage(ReviveAddress::from(addr).inner(), slot.to_be_bytes()).await?;
573-
Ok(bytes.unwrap_or_default().into())
571+
let bytes: B256 = match runtime_api
572+
.get_storage(ReviveAddress::from(addr).inner(), slot.to_be_bytes())
573+
.await
574+
{
575+
Ok(Some(bytes)) => bytes.as_slice().try_into().map_err(|_| {
576+
Error::InternalError("Unable to convert value to 32-byte value".to_string())
577+
})?,
578+
Ok(None) | Err(ClientError::ContractNotFound) => Default::default(),
579+
Err(err) => return Err(Error::ReviveRpc(EthRpcError::ClientError(err))),
580+
};
581+
Ok(bytes)
574582
}
575583

576584
async fn get_code(&self, address: Address, block: Option<BlockId>) -> Result<Bytes> {
@@ -994,15 +1002,58 @@ impl ApiServer {
9941002

9951003
let latest_block = self.latest_block();
9961004

997-
let Some(ReviveAccountInfo { account_type: AccountType::Contract(contract_info), .. }) =
998-
self.backend.read_revive_account_info(latest_block, address)?
999-
else {
1000-
return Ok(());
1005+
let account_id = self.get_account_id(latest_block, address)?;
1006+
1007+
let maybe_system_account_info =
1008+
self.backend.read_system_account_info(latest_block, account_id.clone())?;
1009+
let nonce = maybe_system_account_info.as_ref().map(|info| info.nonce).unwrap_or_default();
1010+
1011+
if maybe_system_account_info.is_none() {
1012+
self.set_frame_system_balance(
1013+
latest_block,
1014+
account_id,
1015+
substrate_runtime::currency::DOLLARS,
1016+
)?;
1017+
}
1018+
1019+
let trie_id = match self.backend.read_revive_account_info(latest_block, address)? {
1020+
// If the account doesn't exist, create one.
1021+
None => {
1022+
let contract_info = new_contract_info(&address, (*KECCAK_EMPTY).into(), nonce);
1023+
let trie_id = contract_info.trie_id.clone();
1024+
1025+
self.backend.inject_revive_account_info(
1026+
latest_block,
1027+
address,
1028+
ReviveAccountInfo {
1029+
account_type: AccountType::Contract(contract_info),
1030+
dust: 0,
1031+
},
1032+
);
1033+
1034+
trie_id
1035+
}
1036+
// If the account is not already a contract account, make it one.
1037+
Some(ReviveAccountInfo { account_type: AccountType::EOA, dust }) => {
1038+
let contract_info = new_contract_info(&address, (*KECCAK_EMPTY).into(), nonce);
1039+
let trie_id = contract_info.trie_id.clone();
1040+
1041+
self.backend.inject_revive_account_info(
1042+
latest_block,
1043+
address,
1044+
ReviveAccountInfo { account_type: AccountType::Contract(contract_info), dust },
1045+
);
1046+
1047+
trie_id
1048+
}
1049+
Some(ReviveAccountInfo {
1050+
account_type: AccountType::Contract(contract_info), ..
1051+
}) => contract_info.trie_id,
10011052
};
10021053

10031054
self.backend.inject_child_storage(
10041055
latest_block,
1005-
contract_info.trie_id.to_vec(),
1056+
trie_id.to_vec(),
10061057
key.to_be_bytes_vec(),
10071058
value.to_vec(),
10081059
);

crates/anvil-polkadot/tests/it/standard_rpc.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -599,6 +599,16 @@ async fn test_get_storage() {
599599
unwrap_response::<()>(node.eth_rpc(EthRequest::SetAutomine(true)).await.unwrap()).unwrap();
600600
let alith = Account::from(subxt_signer::eth::dev::alith());
601601

602+
// Test retrieving the storage of an EOA account (alith)
603+
let stored_value = node.get_storage_at(U256::from(0), alith.address()).await;
604+
assert_eq!(stored_value, 0);
605+
606+
// Test retrieving the storage of a non-existant account.
607+
let random_addr = Address::random();
608+
let stored_value =
609+
node.get_storage_at(U256::from(0), ReviveAddress::from(random_addr).inner()).await;
610+
assert_eq!(stored_value, 0);
611+
602612
let contract_code = get_contract_code("SimpleStorage");
603613
let tx_hash = node.deploy_contract(&contract_code.init, alith.address(), Some(1)).await;
604614
tokio::time::sleep(Duration::from_millis(400)).await;

crates/anvil-polkadot/tests/it/state_injector.rs

Lines changed: 73 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -595,51 +595,81 @@ async fn test_set_storage() {
595595
let mut node = TestNode::new(anvil_node_config.clone(), substrate_node_config).await.unwrap();
596596
let alith = Account::from(subxt_signer::eth::dev::alith());
597597

598-
let contract_code = get_contract_code("SimpleStorage");
599-
let tx_hash = node.deploy_contract(&contract_code.init, alith.address(), None).await;
600-
unwrap_response::<()>(node.eth_rpc(EthRequest::Mine(None, None)).await.unwrap()).unwrap();
601-
tokio::time::sleep(std::time::Duration::from_millis(400)).await;
602-
let receipt = node.get_transaction_receipt(tx_hash).await;
603-
let contract_address = receipt.contract_address.unwrap();
604-
605-
// Check the default value for slot 0.
606-
let result = node
607-
.eth_rpc(EthRequest::EthGetStorageAt(
608-
Address::from(ReviveAddress::new(contract_address)),
609-
U256::from(0),
610-
None,
611-
))
612-
.await
598+
// Set storage of a new random account.
599+
{
600+
let random_addr = Address::random();
601+
602+
let stored_value =
603+
node.get_storage_at(U256::from(0), ReviveAddress::from(random_addr).inner()).await;
604+
assert_eq!(stored_value, 0);
605+
606+
// Set a new value for the slot 0.
607+
unwrap_response::<()>(
608+
node.eth_rpc(EthRequest::SetStorageAt(
609+
random_addr,
610+
U256::from(0),
611+
B256::from(U256::from(511)),
612+
))
613+
.await
614+
.unwrap(),
615+
)
613616
.unwrap();
614-
let hex_string = unwrap_response::<String>(result).unwrap();
615-
let hex_value = hex_string.strip_prefix("0x").unwrap_or(&hex_string);
616-
let stored_value = U256::from_str_radix(hex_value, 16).unwrap();
617-
assert_eq!(stored_value, 0);
618617

619-
// Set a new value for the slot 0.
620-
621-
unwrap_response::<()>(
622-
node.eth_rpc(EthRequest::SetStorageAt(
623-
Address::from(ReviveAddress::new(contract_address)),
624-
U256::from(0),
625-
B256::from(U256::from(511)),
626-
))
627-
.await
628-
.unwrap(),
629-
)
630-
.unwrap();
618+
// Check that the value was updated
619+
let stored_value =
620+
node.get_storage_at(U256::from(0), ReviveAddress::from(random_addr).inner()).await;
621+
assert_eq!(stored_value, 511);
622+
}
623+
624+
// Update the storage of an existing account
625+
{
626+
let contract_code = get_contract_code("SimpleStorage");
627+
let tx_hash = node.deploy_contract(&contract_code.init, alith.address(), None).await;
628+
unwrap_response::<()>(node.eth_rpc(EthRequest::Mine(None, None)).await.unwrap()).unwrap();
629+
tokio::time::sleep(std::time::Duration::from_millis(400)).await;
630+
let receipt = node.get_transaction_receipt(tx_hash).await;
631+
let contract_address = receipt.contract_address.unwrap();
632+
633+
// Check the default value for slot 0.
634+
let stored_value = node.get_storage_at(U256::from(0), contract_address).await;
635+
assert_eq!(stored_value, 0);
636+
637+
// Set a new value for the slot 0.
638+
unwrap_response::<()>(
639+
node.eth_rpc(EthRequest::SetStorageAt(
640+
Address::from(ReviveAddress::new(contract_address)),
641+
U256::from(0),
642+
B256::from(U256::from(511)),
643+
))
644+
.await
645+
.unwrap(),
646+
)
647+
.unwrap();
631648

632-
// Check that the value was updated
633-
let result = node
634-
.eth_rpc(EthRequest::EthGetStorageAt(
635-
Address::from(ReviveAddress::new(contract_address)),
636-
U256::from(0),
637-
None,
638-
))
639-
.await
649+
// Check that the value was updated
650+
let stored_value = node.get_storage_at(U256::from(0), contract_address).await;
651+
assert_eq!(stored_value, 511);
652+
}
653+
654+
// Set storage for a EOA account (Alith).
655+
{
656+
let stored_value = node.get_storage_at(U256::from(0), alith.address()).await;
657+
assert_eq!(stored_value, 0);
658+
659+
// Set a new value for the slot 0.
660+
unwrap_response::<()>(
661+
node.eth_rpc(EthRequest::SetStorageAt(
662+
Address::from(ReviveAddress::new(alith.address())),
663+
U256::from(0),
664+
B256::from(U256::from(511)),
665+
))
666+
.await
667+
.unwrap(),
668+
)
640669
.unwrap();
641-
let hex_string = unwrap_response::<String>(result).unwrap();
642-
let hex_value = hex_string.strip_prefix("0x").unwrap_or(&hex_string);
643-
let stored_value = U256::from_str_radix(hex_value, 16).unwrap();
644-
assert_eq!(stored_value, 511);
670+
671+
// Check that the value was updated
672+
let stored_value = node.get_storage_at(U256::from(0), alith.address()).await;
673+
assert_eq!(stored_value, 511);
674+
}
645675
}

crates/script/src/progress.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ impl ScriptProgress {
171171
progress
172172
}
173173

174-
/// Traverses a set of pendings and either finds receipts, or clears them from
174+
/// Traverses a set of pending transactions and either finds receipts, or clears them from
175175
/// the deployment sequence.
176176
///
177177
/// For each `tx_hash`, we check if it has confirmed. If it has

0 commit comments

Comments
 (0)