-
Notifications
You must be signed in to change notification settings - Fork 982
WIP: pallet-revive: Raise contract size limit to one megabyte and raise call depth to 25 #9267
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
base: master
Are you sure you want to change the base?
Conversation
All GitHub workflows were cancelled due to failure one of the required jobs. |
/cmd bench --runtime dev --pallet pallet_revive |
Command "bench --runtime dev --pallet pallet_revive" has started 🚀 See logs here |
…--pallet pallet_revive'
Command "bench --runtime dev --pallet pallet_revive" has finished ✅ See logs here Subweight results:
Command output:✅ Successful benchmarks of runtimes/pallets: |
@@ -46,6 +51,9 @@ pub const NUM_EVENT_TOPICS: u32 = 4; | |||
/// Maximum size of events (including topics) and storage values. | |||
pub const PAYLOAD_BYTES: u32 = 416; | |||
|
|||
/// The maximum size for calldata and return data. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIRC this value matches the current practical limit on Ethereum. Might point that out here?
This PR changes the contract code limit from roughly 100KiB to exactly 1MiB. It also raises the call stack depth from 5 to 25.
Those limits were in place because of memory constraints within the runtime. We work around them in those ways:
1MiB contracts
This is large enough so that all known contracts won't fail for size issues anymore.
The new limit is also much simpler to understand since it does not depend on the number of instructions. Just those two constraints:
This means:
Call stack depth
5 -> 25
The limit of
5
was problematic because there are use cases which require deeper stacks. With the raise to25
there should be no benign use cases anymore that won't work.Please note that even with the low limit of
25
contracts are not vulnerable to stack depth exhaustion attacks: We do trap the caller's context when the depth limit is reached. This is different from Eth where this error can be handled and failure to do so leaves the contract vulnerable.TODO
We are still missing the actual things that make those bigger contracts possible. However, all the calculations are in place.