Skip to content

Conversation

0xRVE
Copy link
Contributor

@0xRVE 0xRVE commented Aug 27, 2025

Added trace logging for each instruction to evm::run function.
solves #9575

@0xRVE 0xRVE added the T7-smart_contracts This PR/Issue is related to smart contracts. label Aug 27, 2025
@0xRVE
Copy link
Contributor Author

0xRVE commented Aug 27, 2025

/cmd prdoc --audience runtime_dev --bump patch

@0xRVE
Copy link
Contributor Author

0xRVE commented Aug 27, 2025

/cmd bench --runtime dev --pallet pallet_revive

Copy link
Contributor

Command "bench --runtime dev --pallet pallet_revive" has started 🚀 See logs here

Copy link
Contributor

Command "bench --runtime dev --pallet pallet_revive" has finished ✅ See logs here

Subweight results:
File Extrinsic Old New Change [%]
cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/weights/cumulus_pallet_xcmp_queue.rs take_first_concatenated_xcm 3.99us 5.61us +40.55
cumulus/parachains/runtimes/assets/asset-hub-rococo/src/weights/cumulus_pallet_xcmp_queue.rs take_first_concatenated_xcm 4.09us 5.61us +37.11
cumulus/parachains/runtimes/coretime/coretime-rococo/src/weights/cumulus_pallet_xcmp_queue.rs take_first_concatenated_xcm 4.08us 5.58us +36.51
cumulus/parachains/runtimes/people/people-westend/src/weights/cumulus_pallet_xcmp_queue.rs take_first_concatenated_xcm 4.01us 5.40us +34.79
cumulus/parachains/runtimes/assets/asset-hub-westend/src/weights/cumulus_pallet_xcmp_queue.rs take_first_concatenated_xcm 3.92us 5.27us +34.34
substrate/frame/revive/src/weights.rs identity 118.31us 156.77us +32.51
substrate/frame/revive/src/weights.rs seal_call_data_copy 118.25us 156.45us +32.31
cumulus/parachains/runtimes/people/people-rococo/src/weights/cumulus_pallet_xcmp_queue.rs take_first_concatenated_xcm 4.08us 5.37us +31.64
cumulus/parachains/runtimes/collectives/collectives-westend/src/weights/cumulus_pallet_xcmp_queue.rs take_first_concatenated_xcm 4.08us 5.37us +31.43
cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/weights/cumulus_pallet_xcmp_queue.rs take_first_concatenated_xcm 4.15us 5.40us +29.99
cumulus/parachains/runtimes/coretime/coretime-westend/src/weights/cumulus_pallet_xcmp_queue.rs take_first_concatenated_xcm 4.18us 5.14us +23.07
cumulus/pallets/xcmp-queue/src/weights.rs take_first_concatenated_xcm 5.88us 7.00us +19.04
substrate/frame/revive/src/weights.rs seal_copy_to_contract 213.32us 252.08us +18.17
substrate/frame/revive/src/weights.rs seal_return 26.83us 31.58us +17.68
cumulus/parachains/runtimes/collectives/collectives-westend/src/weights/cumulus_pallet_xcmp_queue.rs enqueue_empty_xcmp_message_at 529.53us 592.60us +11.91
cumulus/parachains/runtimes/coretime/coretime-westend/src/weights/cumulus_pallet_xcmp_queue.rs enqueue_empty_xcmp_message_at 528.00us 590.50us +11.84
substrate/frame/revive/src/weights.rs seal_gas_limit 509.00ns 552.00ns +8.45
cumulus/parachains/runtimes/collectives/collectives-westend/src/weights/cumulus_pallet_xcmp_queue.rs enqueue_n_full_pages 19.36ms 20.99ms +8.39
cumulus/parachains/runtimes/coretime/coretime-westend/src/weights/cumulus_pallet_xcmp_queue.rs enqueue_n_full_pages 19.37ms 20.96ms +8.17
cumulus/parachains/runtimes/coretime/coretime-rococo/src/weights/cumulus_pallet_xcmp_queue.rs enqueue_empty_xcmp_message_at 526.75us 569.09us +8.04
cumulus/parachains/runtimes/assets/asset-hub-rococo/src/weights/cumulus_pallet_xcmp_queue.rs enqueue_empty_xcmp_message_at 554.35us 592.30us +6.85
substrate/frame/revive/src/weights.rs seal_gas_price 317.00ns 338.00ns +6.62
cumulus/parachains/runtimes/coretime/coretime-westend/src/weights/cumulus_pallet_xcmp_queue.rs on_idle_good_msg 831.06us 884.24us +6.40
cumulus/parachains/runtimes/people/people-westend/src/weights/cumulus_pallet_xcmp_queue.rs enqueue_empty_xcmp_message_at 554.81us 590.30us +6.40
cumulus/parachains/runtimes/coretime/coretime-rococo/src/weights/cumulus_pallet_xcmp_queue.rs enqueue_n_full_pages 19.45ms 20.61ms +5.95
cumulus/parachains/runtimes/collectives/collectives-westend/src/weights/cumulus_pallet_xcmp_queue.rs on_idle_good_msg 836.06us 882.59us +5.57
substrate/frame/revive/src/weights.rs seal_own_code_hash 336.00ns 354.00ns +5.36
cumulus/parachains/runtimes/coretime/coretime-rococo/src/weights/cumulus_pallet_xcmp_queue.rs on_idle_good_msg 828.06us 870.68us +5.15
substrate/frame/revive/src/weights.rs seal_block_number 358.00ns 340.00ns -5.03
cumulus/parachains/runtimes/people/people-rococo/src/weights/cumulus_pallet_xcmp_queue.rs enqueue_n_empty_xcmp_messages 561.38us 532.67us -5.11
cumulus/parachains/runtimes/coretime/coretime-rococo/src/weights/cumulus_pallet_xcmp_queue.rs enqueue_n_empty_xcmp_messages 560.39us 531.58us -5.14
substrate/frame/revive/src/weights.rs seal_return_data_size 326.00ns 309.00ns -5.21
cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/weights/cumulus_pallet_xcmp_queue.rs enqueue_1000_small_xcmp_messages 592.38us 560.54us -5.37
cumulus/parachains/runtimes/coretime/coretime-westend/src/weights/cumulus_pallet_xcmp_queue.rs enqueue_n_empty_xcmp_messages 562.91us 532.37us -5.43
substrate/frame/revive/src/weights.rs seal_origin 386.00ns 365.00ns -5.44
cumulus/parachains/runtimes/people/people-rococo/src/weights/cumulus_pallet_xcmp_queue.rs enqueue_1000_small_xcmp_messages 591.12us 557.99us -5.60
substrate/frame/revive/src/weights.rs seal_base_fee 347.00ns 327.00ns -5.76
cumulus/parachains/runtimes/assets/asset-hub-westend/src/weights/cumulus_pallet_xcmp_queue.rs enqueue_n_empty_xcmp_messages 564.11us 531.26us -5.82
cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/weights/cumulus_pallet_xcmp_queue.rs enqueue_n_empty_xcmp_messages 563.79us 530.00us -5.99
substrate/frame/revive/src/weights.rs bn128_add 15.94us 14.98us -6.06
cumulus/parachains/runtimes/assets/asset-hub-westend/src/weights/cumulus_pallet_xcmp_queue.rs enqueue_1000_small_xcmp_messages 601.58us 564.83us -6.11
cumulus/pallets/xcmp-queue/src/weights.rs enqueue_1000_small_xcmp_messages 481.00us 448.00us -6.86
cumulus/pallets/xcmp-queue/src/weights.rs enqueue_n_empty_xcmp_messages 544.39us 503.97us -7.43
substrate/frame/revive/src/weights.rs seal_minimum_balance 339.00ns 312.00ns -7.96
substrate/frame/revive/src/weights.rs v2_migration_step 161.88us Added
substrate/frame/revive/src/weights.rs to_account_id 33.09us Added
substrate/frame/revive/src/weights.rs evm_opcode 68.85us Added
substrate/frame/revive/src/weights.rs call_with_pvm_code_per_byte 645.69us Added
substrate/frame/revive/src/weights.rs call_with_evm_code_per_byte 462.10us Added
Command output:

✅ Successful benchmarks of runtimes/pallets:
-- dev: ['pallet_revive']

Copy link
Member

@xermicus xermicus left a comment

Choose a reason for hiding this comment

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

lgtm - I guess this helps a lot when writing and debugging low level tests. Is subject to final review when the base goes into master.

@xermicus
Copy link
Member

xermicus commented Aug 28, 2025

We need the run_plain function in #9414 in order to propagate the ExecError.

Edit: Maybe not and we can get around with error conversions. Ignore this comment for now.

interpreter.bytecode.pc(),
OpCode::new(interpreter.bytecode.opcode())
.map_or("INVALID".to_string(), |x| format!("{:?}", x.info())),
interpreter.stack.top().map_or("None".to_string(), |x| format!("{:#x}", x)),
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, looks good. Is there a way to print more than just the top of the stack? Many functions pop more than one argument. 5 values would be a good limit I think

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not without popping elements, then printing, and then pushing them back.

Robert van Eerdewijk added 2 commits August 28, 2025 16:56
@github-actions github-actions bot requested a review from xermicus August 28, 2025 14:57
Copy link
Contributor

Review required! Latest push from author must always be reviewed

@github-actions github-actions bot requested review from pgherveou and xermicus August 29, 2025 08:01
@paritytech-workflow-stopper
Copy link

All GitHub workflows were cancelled due to failure one of the required jobs.
Failed workflow url: https://github.com/paritytech/polkadot-sdk/actions/runs/17331593533
Failed job name: quick-benchmarks-omni

@athei
Copy link
Member

athei commented Sep 4, 2025

Why so many conditionals? You can just define the function twice. Once for std and once without. You can also move the action inside. Where the no-std version just calls the original run_plain.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T7-smart_contracts This PR/Issue is related to smart contracts.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[pallet-revive] EVM backend: Add tracing when running in REVM interpreter
5 participants