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 base fee API #6964

Merged
merged 8 commits into from
Dec 19, 2024
Merged

[pallet-revive] implement the base fee API #6964

merged 8 commits into from
Dec 19, 2024

Conversation

xermicus
Copy link
Member

@xermicus xermicus commented Dec 19, 2024

This PR implements the base fee syscall API method. Currently this is implemented as a compile time constant in the revive compiler, returning 0. However, since this is an opocde, if we ever need to implement it for compatibility reasons with EIP-1559, it would break already deployed contracts. Thus we provide a syscall method instead.

Signed-off-by: xermicus <[email protected]>
@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 Dec 19, 2024
@xermicus xermicus requested review from pgherveou and athei December 19, 2024 10:40
@xermicus
Copy link
Member Author

bot fmt

@command-bot
Copy link

command-bot bot commented Dec 19, 2024

@xermicus https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7940391 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 17-c042744b-ddd4-4a07-8713-0a1acf7c1ff8 to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Dec 19, 2024

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

@xermicus
Copy link
Member Author

/cmd prdoc --audience runtime_dev --bump minor

@xermicus
Copy link
Member Author

/cmd bench --runtime dev --pallet pallet_revive

@xermicus xermicus enabled auto-merge December 19, 2024 10:50
Copy link

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

Copy link

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

Subweight results:
File Extrinsic Old New Change [%]
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/collectives/collectives-westend/src/weights/pallet_core_fellowship_ambassador_core.rs promote_fast - - ERROR
cumulus/parachains/runtimes/collectives/collectives-westend/src/weights/pallet_core_fellowship_fellowship_core.rs promote_fast - - ERROR
cumulus/parachains/runtimes/collectives/collectives-westend/src/weights/pallet_preimage.rs ensure_updated - - 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_contains_storage 33.98us 37.67us +10.85
substrate/frame/revive/src/weights.rs seal_get_storage 35.00us 38.79us +10.82
substrate/frame/revive/src/weights.rs get_storage_empty 32.91us 36.39us +10.57
substrate/frame/revive/src/weights.rs seal_get_immutable_data 34.32us 37.49us +9.25
substrate/frame/revive/src/weights.rs seal_ecdsa_recover 48.53us 51.62us +6.37
substrate/frame/revive/src/weights.rs seal_balance_of 59.37us 62.46us +5.20
substrate/frame/revive/src/weights.rs seal_call_data_load 297.00ns 312.00ns +5.05
substrate/frame/revive/src/weights.rs seal_value_transferred 305.00ns 288.00ns -5.57
substrate/frame/revive/src/weights.rs seal_own_code_hash 360.00ns 339.00ns -5.83
substrate/frame/revive/src/weights.rs get_transient_storage_full 1.85us 1.73us -6.43
substrate/frame/revive/src/weights.rs seal_block_number 325.00ns 302.00ns -7.08
substrate/frame/revive/src/weights.rs seal_origin 329.00ns 302.00ns -8.21
substrate/frame/revive/src/weights.rs seal_ref_time_left 328.00ns 290.00ns -11.59
substrate/frame/revive/src/weights.rs seal_caller_is_root 327.00ns 282.00ns -13.76
substrate/frame/revive/src/weights.rs seal_address 359.00ns 306.00ns -14.76
substrate/frame/revive/src/weights.rs seal_balance 5.86us 4.97us -15.16
substrate/frame/revive/src/weights.rs seal_now 328.00ns 278.00ns -15.24
substrate/frame/revive/src/weights.rs seal_caller 373.00ns 300.00ns -19.57
substrate/frame/revive/src/weights.rs seal_base_fee 273.00ns Added
polkadot/runtime/westend/src/weights/polkadot_runtime_parachains_disputes_slashing.rs report_dispute_lost 1.59ms Added
Command output:

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

@athei
Copy link
Member

athei commented Dec 19, 2024

It returns an u64 because ref_time is also u64 and the base fee is always less.

Why? Isn't this a balance value which could theoretically exceed u64?

@xermicus
Copy link
Member Author

It returns an u64 because ref_time is also u64 and the base fee is always less.

Why? Isn't this a balance value which could theoretically exceed u64?

Ah yes of course - I just short-circuited it because the gas price is currently 1. But it might exceed it in theory. I'm going to change it to return a u256.

@xermicus
Copy link
Member Author

/cmd bench --runtime dev --pallet pallet_revive

Copy link

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

@athei
Copy link
Member

athei commented Dec 19, 2024

It returns an u64 because ref_time is also u64 and the base fee is always less.

Why? Isn't this a balance value which could theoretically exceed u64?

Ah yes of course - I just short-circuited it because the gas price is currently 1. But it might exceed it in theory. I'm going to change it to return a u256.

Strictly speaking it is capped to u128 which is our balance type everywhere. But it is a generic type. And in these cases we exposed it as u256 in all other places.

@xermicus
Copy link
Member Author

@athei yes I fully agree.

Copy link

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

Subweight results:
File Extrinsic Old New Change [%]
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/collectives/collectives-westend/src/weights/pallet_core_fellowship_ambassador_core.rs promote_fast - - ERROR
cumulus/parachains/runtimes/collectives/collectives-westend/src/weights/pallet_core_fellowship_fellowship_core.rs promote_fast - - ERROR
cumulus/parachains/runtimes/collectives/collectives-westend/src/weights/pallet_preimage.rs ensure_updated - - 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_contains_storage 33.98us 37.62us +10.69
substrate/frame/revive/src/weights.rs seal_get_storage 35.00us 38.64us +10.39
substrate/frame/revive/src/weights.rs seal_gas_price 293.00ns 323.00ns +10.24
substrate/frame/revive/src/weights.rs get_storage_empty 32.91us 36.28us +10.23
substrate/frame/revive/src/weights.rs seal_get_immutable_data 34.32us 37.52us +9.35
substrate/frame/revive/src/weights.rs seal_balance_of 59.37us 62.65us +5.53
substrate/frame/revive/src/weights.rs get_transient_storage_full 1.85us 1.71us -7.46
substrate/frame/revive/src/weights.rs seal_minimum_balance 303.00ns 279.00ns -7.92
substrate/frame/revive/src/weights.rs seal_caller_is_root 327.00ns 299.00ns -8.56
substrate/frame/revive/src/weights.rs seal_value_transferred 305.00ns 274.00ns -10.16
substrate/frame/revive/src/weights.rs seal_now 328.00ns 290.00ns -11.59
substrate/frame/revive/src/weights.rs seal_return 62.36us 54.79us -12.14
substrate/frame/revive/src/weights.rs seal_return_data_size 304.00ns 267.00ns -12.17
substrate/frame/revive/src/weights.rs seal_copy_to_contract 62.23us 53.96us -13.30
substrate/frame/revive/src/weights.rs seal_call_data_size 309.00ns 267.00ns -13.59
substrate/frame/revive/src/weights.rs seal_call_data_load 297.00ns 256.00ns -13.80
substrate/frame/revive/src/weights.rs seal_caller_is_origin 394.00ns 338.00ns -14.21
substrate/frame/revive/src/weights.rs seal_own_code_hash 360.00ns 305.00ns -15.28
substrate/frame/revive/src/weights.rs seal_block_number 325.00ns 274.00ns -15.69
substrate/frame/revive/src/weights.rs seal_ref_time_left 328.00ns 273.00ns -16.77
substrate/frame/revive/src/weights.rs seal_balance 5.86us 4.84us -17.34
substrate/frame/revive/src/weights.rs seal_caller 373.00ns 305.00ns -18.23
substrate/frame/revive/src/weights.rs seal_origin 329.00ns 265.00ns -19.45
substrate/frame/revive/src/weights.rs seal_gas_limit 356.00ns 280.00ns -21.35
substrate/frame/revive/src/weights.rs seal_call_data_copy 39.54us 30.22us -23.59
substrate/frame/revive/src/weights.rs seal_address 359.00ns 271.00ns -24.51
substrate/frame/revive/src/weights.rs seal_base_fee 290.00ns Added
Command output:

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

@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/12413578688
Failed job name: fmt

// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
Copy link
Contributor

Choose a reason for hiding this comment

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

Not demanding to remove this in principle SPDX already fixes this up, and rest is redundant information.

Copy link
Member

Choose a reason for hiding this comment

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

If we want to change the header it needs to be a separate PR changing headers in all files.

// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
Copy link
Member

Choose a reason for hiding this comment

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

If we want to change the header it needs to be a separate PR changing headers in all files.

Signed-off-by: Cyrill Leutwiler <[email protected]>
@xermicus xermicus added this pull request to the merge queue Dec 19, 2024
Merged via the queue into master with commit 243b751 Dec 19, 2024
197 of 201 checks passed
@xermicus xermicus deleted the cl/basefee branch December 19, 2024 16:19
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