fix: estimate gas for bounded substrate accounts#1409
fix: estimate gas for bounded substrate accounts#1409
Conversation
|
Crate versions that have been updated:
Runtime version has been increased. |
There was a problem hiding this comment.
Pull request overview
This PR adjusts HydraDX’s Frontier integration to ensure eth_estimateGas and other RPC calls work correctly when the Frontier rpc-binary-search-estimate feature forces estimate = false, while still preventing on-chain Ethereum transactions from using bound Substrate accounts.
Changes:
- Move the “bound address cannot be used” restriction from the Frontier runtime RPC APIs (
call/create) to self-contained extrinsic pre-dispatch validation for on-chain Ethereum transactions. - Update integration tests to assert RPC calls succeed for bound addresses (for both
estimate=true/false) and that on-chain Ethereum transactions are rejected when the sender is bound. - Expose
AccountExtensionstorage map publicly to enable direct storage setup in integration tests.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
runtime/hydradx/src/lib.rs |
Enforces bound-address restriction at self-contained tx pre-dispatch; removes bound-address restriction from RPC runtime APIs. |
pallets/evm-accounts/src/lib.rs |
Makes AccountExtension storage type public (used by integration tests). |
integration-tests/src/evm.rs |
Updates/extends tests to cover RPC behavior from bound addresses and on-chain tx rejection behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| RuntimeCall::Ethereum(call) => { | ||
| // don't allow on-chain EVM transactions from a bound address | ||
| if EVMAccounts::bound_account_id(*info).is_some() { | ||
| return Some(Err(TransactionValidityError::Invalid(InvalidTransaction::BadSigner))); |
There was a problem hiding this comment.
InvalidTransaction::BadSigner is typically reserved for signature/account mismatches. Here the signature can be valid, but the sender is rejected solely because the address is bound; using BadSigner can be misleading for clients and any error mapping/telemetry. Consider using a more semantically correct validity error (e.g., InvalidTransaction::Call or a Custom code) to represent the bound-address restriction distinctly.
| return Some(Err(TransactionValidityError::Invalid(InvalidTransaction::BadSigner))); | |
| return Some(Err(TransactionValidityError::Invalid(InvalidTransaction::Call))); |
| pub type AccountExtension<T: Config> = StorageMap<_, Blake2_128Concat, EvmAddress, AccountIdLast12Bytes>; | ||
|
|
There was a problem hiding this comment.
Changing AccountExtension from pub(super) to pub exposes the raw storage map as part of the pallet’s Rust API, making it easier for other runtime code to write to it directly and bypass invariants/events (inc_sufficients, Bound event, etc.). If this is only needed for integration tests, prefer a narrowly-scoped test helper (e.g., a function behind a dedicated cfg(feature = "testing")/cfg(test) or a helper in the integration test harness that writes storage by key) to avoid broadening the pallet’s public surface area.
| pub type AccountExtension<T: Config> = StorageMap<_, Blake2_128Concat, EvmAddress, AccountIdLast12Bytes>; | |
| pub(super) type AccountExtension<T: Config> = StorageMap<_, Blake2_128Concat, EvmAddress, AccountIdLast12Bytes>; | |
| #[cfg(any(test, feature = "testing"))] | |
| impl<T: Config> Pallet<T> { | |
| /// Test-only helper for seeding account-extension storage without exposing the | |
| /// raw storage map in the pallet's public production API. | |
| pub fn insert_account_extension_for_testing(evm_address: EvmAddress, account_extension: AccountIdLast12Bytes) { | |
| AccountExtension::<T>::insert(evm_address, account_extension); | |
| } | |
| } |
|
Quick benchmark at commit a46e75c has been executed successfully. |
problem
the frontier fork (
moonbeam-foundation/frontier, branchmoonbeam-polkadot-stable2506) withrpc-binary-search-estimatefeature enabled since #1387with this feature,
eth_estimateGassetsestimate = falseduring its binary search loop to get accurate gas measurements. the original check (if !estimate && ...) assumedestimate = truewould be passed for gas estimation, which was true with the default frontier behavior but not with this feature flag.so after enabling
rpc-binary-search-estimate,eth_estimateGasfrom bound addresses started returningBoundAddressCannotBeUsed— same error aseth_call.fix
moved the
BoundAddressCannotBeUsedcheck from the runtime api (EthereumRuntimeRPCApi::call/create) topre_dispatch_self_contained, which only runs for actual on-chain ethereum transactions.the runtime api is only called from rpc (
eth_call,eth_estimateGas), and with therpc-binary-search-estimatefrontier feature,estimateis alwaysfalse— causing all rpc calls from bound addresses to fail.