-
Notifications
You must be signed in to change notification settings - Fork 7
Anvil: Fix everything to make it work with real assethub westend #508
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: feature/forking
Are you sure you want to change the base?
Anvil: Fix everything to make it work with real assethub westend #508
Conversation
alindima
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm still not convinced that the prefetching is very useful at this point.
We only prefetch a couple of key/value pairs for one account, but then the block mining will probably need many more keys than this.
I wouldn't do premature optimisations unless you saw a clear improvement in testing. Do you have some numbers?
crates/anvil-polkadot/src/substrate_node/lazy_loading/rpc_client.rs
Outdated
Show resolved
Hide resolved
| /// Execute an ethereum transaction and wait for its receipt. | ||
| /// This is useful for forking tests where transaction validation can take time | ||
| /// due to lazy loading of state from the remote chain. | ||
| pub async fn send_transaction_and_wait( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
many tests already have similar behaviour by sending the transaction, mining a block and then doing wait_for_block_with_timeout. Is this any different?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it makes sense if you want a helper that bundles all this functionality into one, but then let's make use of the wait_for_block_with_timeout helper and remove the explicit sleep here. It shouldn't be needed.
Also, this function should work with automine enabled as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1: also, is there any specific reason why you are using this with 120 seconds timeout? It seems too high
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I'm not wrong the block import and the receipt indexing are decoupled, the block commits instantly but the ReceiptProvider indexes receipts asynchronously via a background task.
This means the receipt isn't queryable immediately after send_transaction, even with automine.
I verified this replacing the helper with send_transaction + mine + wait_for_block_with_timeout + get_transaction_receipt (the old pattern) causes all Westend forking tests to fail
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The transaction receipt is available after Mine when Mine succeeds, the problem is that Mine itself probably fails because its inner call to wait_for_hash times out before the receipt indexer finishes processing (you can double-check if this is right by looking at the output of Mine). In this method, you keep polling and producing new blocks which is sub-optimal. Can you please try using a higher timeout inside wait_for_hash? We can use TIMEOUT_DURATION, which is currently set to 30 seconds. We can also choose to use a higher timeout only when forking is enabled and add a comment explaining why do we need a higher timeout in that case.
The improvement is only a few milliseconds in our tests, so we can remove it for now and revisit potential optimizations in future PRs |
| /// Execute an ethereum transaction and wait for its receipt. | ||
| /// This is useful for forking tests where transaction validation can take time | ||
| /// due to lazy loading of state from the remote chain. | ||
| pub async fn send_transaction_and_wait( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1: also, is there any specific reason why you are using this with 120 seconds timeout? It seems too high
crates/anvil-polkadot/src/substrate_node/lazy_loading/backend/forked_lazy_backend.rs
Outdated
Show resolved
Hide resolved
iulianbarbu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The forking tests fail locally for me. Are they passing on your end?
failures:
---- forking::test_fork_can_deploy_contract_from_westend stdout ----
thread 'forking::test_fork_can_deploy_contract_from_westend' panicked at crates/anvil-polkadot/tests/it/forking.rs:997:10:
Contract deployment receipt not found within timeout: RpcError { code: InternalError, message: "Internal error: Failed to submit transaction: RPC error: client error: ErrorObject { code: ServerError(1010), message: \"Invalid Transaction\", data: Some(RawValue(\"Inability to pay some fees (e.g. account balance too low)\")) }", data: None }
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
---- forking::test_fork_impersonate_account_from_westend stdout ----
thread 'forking::test_fork_impersonate_account_from_westend' panicked at crates/anvil-polkadot/tests/it/forking.rs:1070:10:
Impersonated transaction receipt not found within timeout: RpcError { code: InternalError, message: "Internal error: Failed to submit transaction: RPC error: client error: ErrorObject { code: ServerError(1010), message: \"Invalid Transaction\", data: Some(RawValue(\"Inability to pay some fees (e.g. account balance too low)\")) }", data: None }
---- forking::test_fork_state_snapshotting_from_westend stdout ----
thread 'forking::test_fork_state_snapshotting_from_westend' panicked at crates/anvil-polkadot/tests/it/forking.rs:809:10:
Transaction receipt not found within timeout: RpcError { code: InternalError, message: "Internal error: Failed to submit transaction: RPC error: client error: ErrorObject { code: ServerError(1010), message: \"Invalid Transaction\", data: Some(RawValue(\"Inability to pay some fees (e.g. account balance too low)\")) }", data: None }
---- forking::test_fork_eth_get_nonce_from_westend stdout ----
thread 'forking::test_fork_eth_get_nonce_from_westend' panicked at crates/anvil-polkadot/tests/it/forking.rs:740:10:
Transaction receipt not found within timeout: RpcError { code: InternalError, message: "Internal error: Failed to submit transaction: RPC error: client error: ErrorObject { code: ServerError(1010), message: \"Invalid Transaction\", data: Some(RawValue(\"Inability to pay some fees (e.g. account balance too low)\")) }", data: None }
---- forking::test_fork_can_send_tx_from_westend stdout ----
thread 'forking::test_fork_can_send_tx_from_westend' panicked at crates/anvil-polkadot/tests/it/forking.rs:902:10:
Transaction receipt not found within timeout: RpcError { code: InternalError, message: "Internal error: Failed to submit transaction: RPC error: client error: ErrorObject { code: ServerError(1010), message: \"Invalid Transaction\", data: Some(RawValue(\"Inability to pay some fees (e.g. account balance too low)\")) }", data: None }
---- forking::test_fork_eth_get_code_from_westend stdout ----
thread 'forking::test_fork_eth_get_code_from_westend' panicked at crates/anvil-polkadot/tests/it/utils.rs:390:57:
called `Result::unwrap()` on an `Err` value: RpcError { code: InternalError, message: "Internal error: Failed to submit transaction: RPC error: client error: ErrorObject { code: ServerError(1010), message: \"Invalid Transaction\", data: Some(RawValue(\"Inability to pay some fees (e.g. account balance too low)\")) }", data: None }
failures:
forking::test_fork_can_deploy_contract_from_westend
forking::test_fork_can_send_tx_from_westend
forking::test_fork_eth_get_code_from_westend
forking::test_fork_eth_get_nonce_from_westend
forking::test_fork_impersonate_account_from_westend
forking::test_fork_state_snapshotting_from_westend
test result: FAILED. 114 passed; 6 failed; 0 ignored; 0 measured; 0 filtered out; finished in 89.72s
Yeah I noticed the same yesterday, the used to pass but I'm getting some 404 now, I'm working on debugging this, will let u know |
I found the issue — it’s related to the latest runtime update on Westend, so I need to update polkadot-sdk to fix it. Thanks! |
Fixed |
| // Use dynamic transaction building to ensure the correct pallet index is used. | ||
| // The metadata in self.api comes from the runtime's WASM (via runtime API call), | ||
| // which is the forked chain's WASM when forking. This ensures correct pallet indices. | ||
| let payload_value = DynamicValue::from_bytes(transaction.0.clone()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| let payload_value = DynamicValue::from_bytes(transaction.0.clone()); | |
| let payload_value = DynamicValue::from_bytes(transaction.0); |
| /// Execute an ethereum transaction and wait for its receipt. | ||
| /// This is useful for forking tests where transaction validation can take time | ||
| /// due to lazy loading of state from the remote chain. | ||
| pub async fn send_transaction_and_wait( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The transaction receipt is available after Mine when Mine succeeds, the problem is that Mine itself probably fails because its inner call to wait_for_hash times out before the receipt indexer finishes processing (you can double-check if this is right by looking at the output of Mine). In this method, you keep polling and producing new blocks which is sub-optimal. Can you please try using a higher timeout inside wait_for_hash? We can use TIMEOUT_DURATION, which is currently set to 30 seconds. We can also choose to use a higher timeout only when forking is enabled and add a comment explaining why do we need a higher timeout in that case.
Description
This PR enables forking from production chains like Westend Asset Hub by implementing dynamic transaction building and batch storage prefetching to address pallet index mismatches and reduce RPC latency.
Changes
Dynamic Transaction Building
When forking from a chain with a different runtime configuration, the hardcoded pallet index used by
subxt_client::tx().revive().eth_transact()may not match the forked chain's pallet ordering, causing transaction failures. This PR introduces dynamic transaction building using subxt'sdynamic_tx()which reads pallet indices from the runtime metadata at execution time, ensuring correct encoding regardless of the forked chain's configuration.Batch Storage Prefetching
Each storage read from a remote chain requires an individual RPC call, causing significant latency during transaction validation. This PR implements batch prefetching using
state_queryStorageAtto fetch multiple storage keys in a single RPC call. Before validating an Ethereum transaction, the sender's account info is prefetched to avoid sequential RPC round-trips.Tests
send_transaction_and_wait()anddeploy_contract_and_wait()helpers that poll for transaction receipts with configurable timeouts, essential for reliable testing against remote networks with variable block timesforking-testsfeature flag - forking tests now run without special configurationAll forking tests pass against real Westend Asset Hub