Skip to content
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

[pallet-revive] implement the block author API #7198

Merged
merged 36 commits into from
Jan 29, 2025
Merged

[pallet-revive] implement the block author API #7198

merged 36 commits into from
Jan 29, 2025

Conversation

xermicus
Copy link
Member

This PR implements the block author API method. Runtimes ought to implement it such that it corresponds to the coinbase EVM opcode.

@xermicus xermicus added R0-silent Changes should not be mentioned in any release notes T7-smart_contracts This PR/Issue is related to smart contracts. labels Jan 16, 2025
@xermicus xermicus requested review from pgherveou and athei January 16, 2025 08:53
@xermicus
Copy link
Member Author

bot fmt

@command-bot
Copy link

command-bot bot commented Jan 16, 2025

@xermicus https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/8035186 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh". Check out https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 11-c02238d4-dd11-48c2-a2d2-05be540b0c2a to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Jan 16, 2025

@xermicus Command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh" has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/8035186 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/8035186/artifacts/download.

prdoc/pr_7198.prdoc Outdated Show resolved Hide resolved
substrate/frame/revive/src/exec.rs Show resolved Hide resolved
substrate/frame/revive/src/wasm/runtime.rs Show resolved Hide resolved
substrate/frame/revive/src/weights.rs Outdated Show resolved Hide resolved
substrate/frame/revive/src/exec.rs Show resolved Hide resolved
Comment on lines 895 to 896
digest.push(DigestItem::PreRuntime([1, 2, 3, 4], 123.encode()));
digest.push(DigestItem::Seal([1, 2, 4, 4], 123.encode()));
Copy link
Member

Choose a reason for hiding this comment

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

Is this a realistic amount of messages to be expected in this Vec? Is this the worst case?

Copy link
Member Author

@xermicus xermicus Jan 22, 2025

Choose a reason for hiding this comment

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

I see #[pallet::unbounded] over Digest and log is just alloc::vec::Vec. I don't see anything indicating this to be bounded.

@xermicus xermicus requested a review from athei January 27, 2025 10:45
@github-actions github-actions bot deleted a comment from xermicus Jan 27, 2025
@athei
Copy link
Member

athei commented Jan 27, 2025

You might need to add

frame = { workspace = true, features = ["runtime"] }

To the revive Cargo.toml after the omni bencher has updated the weight file. The new template assumes that you import the umbrella crate under this name.

@athei
Copy link
Member

athei commented Jan 27, 2025

We use the dev weights in asset-hub-westend. This is why it is failing. Only run for dev.

@xermicus xermicus enabled auto-merge January 28, 2025 09:07
@xermicus xermicus disabled auto-merge January 28, 2025 09:07
@github-actions github-actions bot deleted a comment from xermicus Jan 28, 2025
@xermicus
Copy link
Member Author

/cmd bench-omni --runtime dev --pallet pallet_revive --fail-fast --clean

@github-actions github-actions bot deleted a comment from xermicus Jan 29, 2025
Copy link
Contributor

Command "bench-omni --runtime dev --pallet pallet_revive --fail-fast --clean" has started 🚀 See logs here

Copy link
Contributor

Command "bench-omni --runtime dev --pallet pallet_revive --fail-fast --clean" has finished ✅ See logs here

Subweight results:
File Extrinsic Old New Change [%]
cumulus/parachains/runtimes/collectives/collectives-westend/src/weights/pallet_preimage.rs ensure_updated - - ERROR
cumulus/pallets/collator-selection/src/weights.rs leave_intent - - ERROR
cumulus/pallets/collator-selection/src/weights.rs new_session - - ERROR
cumulus/pallets/collator-selection/src/weights.rs register_as_candidate - - ERROR
cumulus/pallets/collator-selection/src/weights.rs set_invulnerables - - ERROR
cumulus/pallets/collator-selection/src/weights.rs take_candidate_slot - - ERROR
cumulus/pallets/collator-selection/src/weights.rs update_bond - - ERROR
cumulus/parachains/runtimes/assets/asset-hub-rococo/src/weights/pallet_collator_selection.rs take_candidate_slot - - ERROR
cumulus/parachains/runtimes/assets/asset-hub-rococo/src/weights/pallet_collator_selection.rs update_bond - - ERROR
cumulus/parachains/runtimes/assets/asset-hub-westend/src/weights/pallet_collator_selection.rs take_candidate_slot - - ERROR
cumulus/parachains/runtimes/assets/asset-hub-westend/src/weights/pallet_collator_selection.rs update_bond - - ERROR
cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/weights/pallet_collator_selection.rs take_candidate_slot - - ERROR
cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/weights/pallet_collator_selection.rs update_bond - - ERROR
cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/weights/pallet_collator_selection.rs take_candidate_slot - - ERROR
cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/weights/pallet_collator_selection.rs update_bond - - ERROR
cumulus/parachains/runtimes/collectives/collectives-westend/src/weights/pallet_collator_selection.rs take_candidate_slot - - ERROR
cumulus/parachains/runtimes/collectives/collectives-westend/src/weights/pallet_collator_selection.rs update_bond - - ERROR
cumulus/parachains/runtimes/people/people-rococo/src/weights/pallet_collator_selection.rs take_candidate_slot - - ERROR
cumulus/parachains/runtimes/people/people-rococo/src/weights/pallet_collator_selection.rs update_bond - - ERROR
cumulus/parachains/runtimes/people/people-westend/src/weights/pallet_collator_selection.rs take_candidate_slot - - ERROR
cumulus/parachains/runtimes/people/people-westend/src/weights/pallet_collator_selection.rs update_bond - - ERROR
polkadot/runtime/westend/src/weights/pallet_preimage.rs ensure_updated - - ERROR
substrate/frame/election-provider-support/src/weights.rs phragmen - - ERROR
substrate/frame/election-provider-support/src/weights.rs phragmms - - ERROR
substrate/frame/revive/src/weights.rs seal_call_data_copy 30.61us 39.53us +29.12
substrate/frame/revive/src/weights.rs seal_copy_to_contract 53.29us 62.65us +17.56
substrate/frame/revive/src/weights.rs seal_return 53.56us 62.75us +17.15
substrate/frame/revive/src/weights.rs seal_block_hash 28.82us 27.37us -5.02
substrate/frame/revive/src/weights.rs get_storage_full 68.76us 65.11us -5.32
substrate/frame/revive/src/weights.rs seal_ecdsa_recover 50.49us 47.75us -5.43
substrate/frame/revive/src/weights.rs seal_value_transferred 309.00ns 292.00ns -5.50
substrate/frame/revive/src/weights.rs seal_delegate_call 107.50us 100.96us -6.08
substrate/frame/revive/src/weights.rs seal_code_size 65.17us 61.19us -6.11
substrate/frame/revive/src/weights.rs seal_balance_of 62.64us 58.77us -6.19
substrate/frame/revive/src/weights.rs seal_deposit_event 5.51us 5.12us -7.02
substrate/frame/revive/src/weights.rs seal_weight_left 730.00ns 675.00ns -7.53
substrate/frame/revive/src/weights.rs seal_set_transient_storage 2.79us 2.58us -7.64
substrate/frame/revive/src/weights.rs on_process_deletion_queue_batch 27.96us 25.74us -7.92
substrate/frame/revive/src/weights.rs set_transient_storage_full 1.97us 1.79us -9.34
substrate/frame/revive/src/weights.rs seal_get_immutable_data 37.46us 33.95us -9.36
substrate/frame/revive/src/weights.rs seal_minimum_balance 315.00ns 284.00ns -9.84
substrate/frame/revive/src/weights.rs seal_take_transient_storage 2.80us 2.52us -9.89
substrate/frame/revive/src/weights.rs seal_to_account_id 35.09us 31.41us -10.48
substrate/frame/revive/src/weights.rs seal_code_hash 36.63us 32.74us -10.60
substrate/frame/revive/src/weights.rs get_transient_storage_empty 1.59us 1.43us -10.66
substrate/frame/revive/src/weights.rs seal_clear_transient_storage 2.60us 2.33us -10.67
substrate/frame/revive/src/weights.rs seal_is_contract 35.55us 31.71us -10.79
substrate/frame/revive/src/weights.rs seal_weight_to_fee 1.48us 1.31us -11.24
substrate/frame/revive/src/weights.rs set_transient_storage_empty 1.64us 1.45us -11.67
substrate/frame/revive/src/weights.rs get_transient_storage_full 1.78us 1.57us -11.90
substrate/frame/revive/src/weights.rs seal_gas_limit 331.00ns 290.00ns -12.39
substrate/frame/revive/src/weights.rs dispatch_as_fallback_account 64.28us 56.25us -12.50
substrate/frame/revive/src/weights.rs seal_contains_transient_storage 2.07us 1.77us -14.22
substrate/frame/revive/src/weights.rs seal_get_transient_storage 2.33us 2.00us -14.47
substrate/frame/revive/src/weights.rs get_storage_empty 36.57us 31.25us -14.55
substrate/frame/revive/src/weights.rs seal_get_storage 38.82us 33.05us -14.87
substrate/frame/revive/src/weights.rs seal_block_number 314.00ns 267.00ns -14.97
substrate/frame/revive/src/weights.rs seal_contains_storage 37.98us 32.26us -15.05
substrate/frame/revive/src/weights.rs seal_caller_is_root 332.00ns 282.00ns -15.06
substrate/frame/revive/src/weights.rs seal_now 316.00ns 264.00ns -16.46
substrate/frame/revive/src/weights.rs seal_own_code_hash 328.00ns 274.00ns -16.46
substrate/frame/revive/src/weights.rs seal_return_data_size 310.00ns 257.00ns -17.10
substrate/frame/revive/src/weights.rs seal_address 336.00ns 274.00ns -18.45
substrate/frame/revive/src/weights.rs seal_origin 329.00ns 264.00ns -19.76
substrate/frame/revive/src/weights.rs seal_gas_price 314.00ns 249.00ns -20.70
substrate/frame/revive/src/weights.rs seal_base_fee 341.00ns 266.00ns -21.99
substrate/frame/revive/src/weights.rs rollback_transient_storage 1.41us 1.09us -22.51
substrate/frame/revive/src/weights.rs seal_call_data_size 338.00ns 261.00ns -22.78
substrate/frame/revive/src/weights.rs seal_ref_time_left 344.00ns 263.00ns -23.55
substrate/frame/revive/src/weights.rs seal_caller 378.00ns 278.00ns -26.46
substrate/frame/revive/src/weights.rs seal_call_data_load 365.00ns 267.00ns -26.85
substrate/frame/revive/src/weights.rs seal_block_author 38.91us Added
Command output:

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

@xermicus
Copy link
Member Author

xermicus commented Jan 29, 2025

The expect in the benchmark makes this brittle, I removed it. Apparently not all benchmarking runtimes ought to implement this (for example with the dev runtime there are no Validators).

@xermicus xermicus enabled auto-merge January 29, 2025 09:04
@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/13027380445
Failed job name: test-linux-stable-no-try-runtime

@xermicus xermicus added this pull request to the merge queue Jan 29, 2025
Merged via the queue into master with commit 2d53238 Jan 29, 2025
202 of 206 checks passed
@xermicus xermicus deleted the cl/coinbase branch January 29, 2025 11:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
R0-silent Changes should not be mentioned in any release notes T7-smart_contracts This PR/Issue is related to smart contracts.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants