-
Notifications
You must be signed in to change notification settings - Fork 8
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
upgrade teerex refactoring #233
Conversation
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.
Changes look good to me
pallet bounties benchmark failed, but that's out of scope and we leave the old weights Input("Benchmark pallet_bounties::spend_funds failed: TooManyQueued") |
.saturating_add(Weight::from_parts(0, 3859)) | ||
// Measured: `451` | ||
// Estimated: `3916` | ||
// Minimum execution time: 2_821_014_000 picoseconds. |
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.
I think that is close enough given different benchmarking machines
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.
Yes, I agree.
// Proof Size summary in bytes: | ||
// Measured: `412` | ||
// Estimated: `3877` | ||
// Minimum execution time: 41_875_000 picoseconds. |
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.
that's the platform bottleneck in terms of future congestion. benchmark is reasonably close
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.
LGTM
.saturating_add(Weight::from_parts(0, 3859)) | ||
// Measured: `451` | ||
// Estimated: `3916` | ||
// Minimum execution time: 2_821_014_000 picoseconds. |
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.
Yes, I agree.
closes #221