Skip to content

client fix: filter buggy transaction not included in the ethereum block#3267

Merged
RomarQ merged 10 commits intomasterfrom
elois-client-tracing-filter-buggy-tx
May 13, 2025
Merged

client fix: filter buggy transaction not included in the ethereum block#3267
RomarQ merged 10 commits intomasterfrom
elois-client-tracing-filter-buggy-tx

Conversation

@librelois
Copy link
Copy Markdown
Collaborator

@librelois librelois commented Apr 23, 2025

⚠️ Breaking Changes ⚠️

Ethereum-XCM transactions that revert are no longer included in block trace.

What does it do?

Filter out transactions generated in the trace but not included in the Ethereum block.

When an eth-xcm transaction reverts, it is not included in the Ethereum block.

Filtering out these buggy transactions ensures consistency between the debug_traceBlock* and eth_getBlock* RPC methods.

The implementation require to keep track of a tx_position_offset and increment it each time we encounter a trace taht doesn't match the expected status (from field are not equal).

Theoretically, it's might be possible to have a buggy transaction and a successful eth-xcm transaction in a raw that have the same caller (=from field), but in practice it never happens and this fix is only for old blocks.

What important points should reviewers know?

This PR add a tracing test test/suites/tracing-tests/test-trace-erc20-xcm-2.ts.

Is there something left for follow-up PRs?

What alternative implementations were considered?

Are there relevant PRs or issues in other repositories (Substrate, Polkadot, Frontier, Cumulus)?

What value does it bring to the blockchain users?

@librelois librelois added B5-clientnoteworthy Changes should be mentioned in any downstream projects' release notes D2-notlive PR doesn't change runtime code (so can't be audited) breaking Needs to be mentioned in breaking changes labels Apr 23, 2025
@librelois librelois requested a review from RomarQ April 23, 2025 11:44
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 23, 2025

WASM runtime size check:

Compared to target branch

Moonbase runtime: 2372 KB (-56 KB) ✅

Moonbeam runtime: 2356 KB (-56 KB) ✅

Moonriver runtime: 2348 KB (-60 KB) ✅

Compared to latest release (runtime-3600)

Moonbase runtime: 2372 KB (+416 KB compared to latest release) ⚠️

Moonbeam runtime: 2356 KB (+412 KB compared to latest release) ⚠️

Moonriver runtime: 2348 KB (+408 KB compared to latest release) ⚠️

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 23, 2025

Coverage Report

@@                           Coverage Diff                            @@
##           master   elois-client-tracing-filter-buggy-tx      +/-   ##
========================================================================
+ Coverage   74.79%                                 75.11%   +0.32%     
+ Files         400                                    406       +6     
- Lines      101212                                 101138      -74     
========================================================================
+ Hits        75696                                  75960     +264     
- Misses      25516                                  25178     -338     
Files Changed Coverage
/client/evm-tracing/src/listeners/call_list.rs 77.36% (-0.03%) 🔽
/node/cli/src/cli.rs 29.79% (+0.32%) 🔼
/node/cli/src/command.rs 17.12% (+0.11%) 🔼
/node/service/src/chain_spec/moonbase.rs 46.22% (-11.67%) 🔽
/node/service/src/chain_spec/moonbeam.rs 0.00% (-23.24%) 🔽
/node/service/src/chain_spec/moonriver.rs 0.00% (-23.24%) 🔽
/node/service/src/lib.rs 52.97% (-8.83%) 🔽
/node/service/src/rpc.rs 83.33% (-1.67%) 🔽
/pallets/asset-manager/src/lib.rs 79.09% (+0.91%) 🔼
/pallets/asset-manager/src/mock.rs 71.05% (+7.89%) 🔼
/pallets/erc20-xcm-bridge/src/lib.rs 13.29% (-0.48%) 🔽
/pallets/erc20-xcm-bridge/src/mock.rs 66.67% (+53.34%) 🔼
/pallets/ethereum-xcm/src/mock.rs 78.40% (+4.00%) 🔼
/pallets/moonbeam-foreign-assets/src/evm.rs 64.24% (+1.07%) 🔼
/pallets/moonbeam-foreign-assets/src/lib.rs 79.55% (+0.14%) 🔼
/pallets/moonbeam-foreign-assets/src/mock.rs 91.58% (+9.47%) 🔼
/pallets/moonbeam-lazy-migrations/src/foreign_asset.rs 94.97% (-0.06%) 🔽
/pallets/moonbeam-lazy-migrations/src/lib.rs 81.58% (-1.55%) 🔽
/pallets/moonbeam-lazy-migrations/src/mock.rs 58.12% (+9.40%) 🔼
/pallets/moonbeam-orbiters/src/lib.rs 75.49% (+0.78%) 🔼
/pallets/moonbeam-orbiters/src/mock.rs 90.67% (+8.00%) 🔼
/pallets/moonbeam-xcm-benchmarks/src/weights/generic.rs 2.35% (-0.24%) 🔽
/pallets/moonbeam-xcm-benchmarks/src/weights/mod.rs 9.09% (-0.86%) 🔽
/pallets/parachain-staking/src/inflation.rs 87.66% (+0.56%) 🔼
/pallets/parachain-staking/src/lib.rs 91.99% (+0.08%) 🔼
/pallets/parachain-staking/src/mock.rs 98.44% (+0.68%) 🔼
/pallets/parachain-staking/src/set.rs 39.47% (+1.01%) 🔼
/pallets/parachain-staking/src/tests.rs 90.90% (-0.06%) 🔽
/pallets/proxy-genesis-companion/src/lib.rs 90.48% (+4.77%) 🔼
/pallets/proxy-genesis-companion/src/mock.rs 86.44% (+13.11%) 🔼
/pallets/xcm-bridge/src/exporter.rs 92.44% (-0.02%) 🔽
/pallets/xcm-bridge/src/lib.rs 77.68% (-0.13%) 🔽
/pallets/xcm-bridge/src/mock.rs 78.48% (+2.10%) 🔼
/pallets/xcm-bridge-router/src/mock.rs 84.27% (+4.92%) 🔼
/pallets/xcm-transactor/src/lib.rs 89.62% (+0.16%) 🔼
/pallets/xcm-transactor/src/mock.rs 69.81% (+3.76%) 🔼
/pallets/xcm-weight-trader/src/lib.rs 87.29% (-1.60%) 🔽
/pallets/xcm-weight-trader/src/mock.rs 94.55% (+10.91%) 🔼
/precompiles/assets-erc20/src/mock.rs 89.80% (+16.33%) 🔼
/precompiles/author-mapping/src/mock.rs 100.00% (+20.51%) 🔼
/precompiles/balances-erc20/src/mock.rs 100.00% (+12.50%) 🔼
/precompiles/batch/src/mock.rs 100.00% (+14.63%) 🔼
/precompiles/call-permit/src/mock.rs 100.00% (+15.79%) 🔼
/precompiles/collective/src/mock.rs 71.23% (+12.90%) 🔼
/precompiles/conviction-voting/src/lib.rs 93.18% (+2.73%) 🔼
/precompiles/conviction-voting/src/mock.rs 71.43% (+5.50%) 🔼
/precompiles/crowdloan-rewards/src/mock.rs 100.00% (+9.57%) 🔼
/precompiles/crowdloan-rewards/src/tests.rs 98.35% (-0.06%) 🔽
/precompiles/gmp/src/mock.rs 26.77% (+6.30%) 🔼
/precompiles/identity/src/lib.rs 90.73% (+0.53%) 🔼
/precompiles/identity/src/mock.rs 93.48% (+15.22%) 🔼
/precompiles/parachain-staking/src/mock.rs 96.72% (+5.74%) 🔼
/precompiles/precompile-registry/src/mock.rs 100.00% (+15.38%) 🔼
/precompiles/preimage/src/mock.rs 100.00% (+18.92%) 🔼
/precompiles/proxy/src/mock.rs 81.82% (+8.49%) 🔼
/precompiles/randomness/src/mock.rs 87.93% (+14.25%) 🔼
/precompiles/referenda/src/mock.rs 75.90% (-1.37%) 🔽
/precompiles/relay-data-verifier/src/mock.rs 96.05% (+10.52%) 🔼
/precompiles/relay-encoder/src/mock.rs 35.51% (+8.41%) 🔼
/precompiles/xcm-transactor/src/mock.rs 74.62% (+6.95%) 🔼
/precompiles/xcm-utils/src/mock.rs 91.35% (+15.52%) 🔼
/precompiles/xtokens/src/lib.rs 96.09% (-0.05%) 🔽
/precompiles/xtokens/src/mock.rs 59.40% (+6.02%) 🔼
/primitives/account/src/lib.rs 61.42% (-3.42%) 🔽
/primitives/bridge/xcm-bridge/src/lib.rs 91.24% (+0.19%) 🔼
/primitives/rpc/evm-tracing-events/src/lib.rs 34.78% (-2.72%) 🔽
/primitives/storage-proof/src/tests.rs 97.56% (-0.02%) 🔽
/primitives/xcm/src/asset_id_conversions.rs 47.83% (+0.21%) 🔼
/runtime/common/src/apis.rs 85.32% (+56.23%) 🔼
/runtime/common/src/impl_moonbeam_xcm_call.rs 95.00% (+0.26%) 🔼
/runtime/common/src/impl_moonbeam_xcm_call_tracing.rs 38.98% (+11.32%) 🔼
/runtime/common/src/impl_xcm_evm_runner.rs 61.36% (+2.71%) 🔼
/runtime/moonbase/src/genesis_config_preset.rs 66.67% (-0.15%) 🔽
/runtime/moonbase/src/governance/origins.rs 46.67% (-18.33%) 🔽
/runtime/moonbase/src/lib.rs 53.19% (+3.44%) 🔼
/runtime/moonbase/src/weights/pallet_collective_open_tech_committee.rs 9.94% (-1.39%) 🔽
/runtime/moonbase/src/weights/pallet_collective_treasury_council.rs 10.18% (-1.46%) 🔽
/runtime/moonbase/src/xcm_config.rs 59.62% (+0.97%) 🔼
/runtime/moonbase/tests/common/mod.rs 97.07% (-0.03%) 🔽
/runtime/moonbase/tests/integration_test.rs 99.42% (-0.01%) 🔽
/runtime/moonbase/tests/xcm_mock/parachain.rs 68.10% (+7.03%) 🔼
/runtime/moonbase/tests/xcm_mock/relay_chain.rs 62.50% (+10.89%) 🔼
/runtime/moonbase/tests/xcm_mock/statemint_like.rs 70.42% (+7.04%) 🔼
/runtime/moonbeam/src/governance/origins.rs 46.67% (-18.33%) 🔽
/runtime/moonbeam/src/lib.rs 51.03% (+6.60%) 🔼
/runtime/moonbeam/src/weights/pallet_collective_open_tech_committee.rs 9.94% (-1.39%) 🔽
/runtime/moonbeam/src/weights/pallet_collective_treasury_council.rs 10.18% (-1.46%) 🔽
/runtime/moonbeam/src/xcm_config.rs 57.69% (+0.96%) 🔼
/runtime/moonbeam/tests/common/mod.rs 95.10% (-0.06%) 🔽
/runtime/moonbeam/tests/integration_test.rs 99.42% (-0.01%) 🔽
/runtime/moonbeam/tests/xcm_mock/parachain.rs 67.74% (+6.67%) 🔼
/runtime/moonbeam/tests/xcm_mock/relay_chain.rs 62.50% (+10.89%) 🔼
/runtime/moonbeam/tests/xcm_mock/statemint_like.rs 70.42% (+7.04%) 🔼
/runtime/moonriver/src/governance/origins.rs 46.67% (-18.33%) 🔽
/runtime/moonriver/src/lib.rs 50.78% (+6.13%) 🔼
/runtime/moonriver/src/weights/pallet_collective_open_tech_committee.rs 9.83% (-1.50%) 🔽
/runtime/moonriver/src/weights/pallet_collective_treasury_council.rs 10.18% (-1.46%) 🔽
/runtime/moonriver/src/xcm_config.rs 57.69% (+0.96%) 🔼
/runtime/moonriver/tests/common/mod.rs 95.21% (-0.04%) 🔽
/runtime/moonriver/tests/xcm_mock/parachain.rs 68.82% (+7.03%) 🔼
/runtime/moonriver/tests/xcm_mock/relay_chain.rs 62.50% (+10.89%) 🔼
/runtime/moonriver/tests/xcm_mock/statemine_like.rs 70.42% (+7.04%) 🔼

Coverage generated Tue May 13 17:11:52 UTC 2025

@RomarQ RomarQ mentioned this pull request Apr 23, 2025
13 tasks
@librelois librelois requested a review from RomarQ April 23, 2025 12:59
Copy link
Copy Markdown
Contributor

@RomarQ RomarQ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are conditions where the traces from XCM calls will be at index 0. For example, when calling PolkadotXcm.transferAssets. For those situations, this fix will not work.

Sandwich scenario:

Trace 1 (XCM Originated) ◀️ `PolkadotXcm.transferAssets` call
Trace 2 (Standard ethereum call)
Trace 3 (XCM Originated) ◀️ XCM message processed by the `on_idle` hook in the `MessageQueue`.

@librelois librelois force-pushed the elois-client-tracing-filter-buggy-tx branch from 02a4e0c to 1f73907 Compare April 28, 2025 14:12
@librelois librelois force-pushed the elois-client-tracing-filter-buggy-tx branch from 8e8ed9f to f12fefc Compare April 30, 2025 14:32
@librelois librelois requested a review from RomarQ April 30, 2025 14:44
RomarQ
RomarQ previously approved these changes May 2, 2025
Copy link
Copy Markdown
Contributor

@RomarQ RomarQ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pnpm check:fix should fix the CI

stiiifff
stiiifff previously approved these changes May 5, 2025
@RomarQ
Copy link
Copy Markdown
Contributor

RomarQ commented May 12, 2025

This can be merged once the conflicts and Biome checks are fixed.

@librelois librelois dismissed stale reviews from stiiifff and RomarQ via 0842f21 May 12, 2025 14:04
@librelois librelois requested review from a team as code owners May 12, 2025 14:04
Copy link
Copy Markdown
Contributor

@RomarQ RomarQ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR expectedly reverts #3258

Since tracing tests are failing on master, we do not know if the tests being added here are passing. We should be able to confirm once #3224 is merged

@RomarQ RomarQ added the A8-mergeoncegreen Pull request is reviewed well. label May 12, 2025
@RomarQ RomarQ merged commit 47d4d9f into master May 13, 2025
36 checks passed
@RomarQ RomarQ deleted the elois-client-tracing-filter-buggy-tx branch May 13, 2025 17:24
RomarQ added a commit that referenced this pull request May 22, 2025
RomarQ added a commit that referenced this pull request May 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A8-mergeoncegreen Pull request is reviewed well. B5-clientnoteworthy Changes should be mentioned in any downstream projects' release notes breaking Needs to be mentioned in breaking changes D2-notlive PR doesn't change runtime code (so can't be audited)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants