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

fix unfairness on proof size check estimation #2946

Merged
merged 18 commits into from
Oct 7, 2024

Conversation

ahmadkaouk
Copy link
Contributor

@ahmadkaouk ahmadkaouk commented Sep 12, 2024

What does it do?

It points frontier and moonbeam to the latest pin in stable, which includes this commit.

What important points reviewers should know?

  • The check to know if an account is an EOA or a smart contract is now done against AccountCodesMetadata, and for that reason the proof size has decreased in some use cases. Those use caes broke some tests that were fixed with more suitable amounts of proof size.
  • batch_not_callable_by_smart_contract was removed because the same functionality is covered by batch_is_not_callable_by_dummy_code

Is there something left for follow-up PRs?

What alternative implementations were considered?

Are there relevant PRs or issues in other repositories (Substrate, Polkadot, Frontier, Cumulus)?

What value does it bring to the blockchain users?

@ahmadkaouk ahmadkaouk added B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes not-breaking Does not need to be mentioned in breaking changes labels Sep 12, 2024
Copy link
Contributor

github-actions bot commented Sep 12, 2024

WASM runtime size check:

Compared to target branch

Moonbase runtime: 2280 KB (no changes) ✅

Moonbeam runtime: 2236 KB (no changes) ✅

Moonriver runtime: 2256 KB (no changes) ✅

Compared to latest release (runtime-3200)

Moonbase runtime: 2280 KB (+320 KB compared to latest release) ⚠️

Moonbeam runtime: 2236 KB (+312 KB compared to latest release) ⚠️

Moonriver runtime: 2256 KB (+332 KB compared to latest release) ⚠️

@ahmadkaouk ahmadkaouk added D3-trivial PR contains trivial changes in a runtime directory that do not require an audit D5-nicetohaveaudit⚠️ PR contains trivial changes to logic that should be properly reviewed. and removed D3-trivial PR contains trivial changes in a runtime directory that do not require an audit labels Sep 13, 2024
librelois
librelois previously approved these changes Sep 13, 2024
@noandrea noandrea changed the title test: Add native transfer test fix unfairness on proof size check estimation Sep 30, 2024
@noandrea noandrea added the A8-mergeoncegreen Pull request is reviewed well. label Sep 30, 2024
Copy link
Contributor

github-actions bot commented Oct 2, 2024

Coverage Report

@@                        Coverage Diff                         @@
##           master   ahmad-evm-native-transfer-test      +/-   ##
==================================================================
- Coverage   79.13%                           79.12%   -0.01%     
  Files         303                              303              
- Lines       88050                            88015      -35     
==================================================================
- Hits        69675                            69637      -38     
+ Misses      18375                            18378       +3     
Files Changed Coverage
/precompiles/batch/src/tests.rs 98.43% (-0.38%) 🔽

Coverage generated Mon Oct 7 15:22:45 UTC 2024

@gonzamontiel gonzamontiel marked this pull request as ready for review October 3, 2024 09:43
Cargo.lock Outdated Show resolved Hide resolved
RomarQ
RomarQ previously requested changes Oct 3, 2024
Copy link
Contributor

@RomarQ RomarQ left a comment

Choose a reason for hiding this comment

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

We should revert the Cargo.lock changes. This PR should only update frontier and moonkit dependencies

librelois
librelois previously approved these changes Oct 3, 2024
@librelois librelois merged commit 55a2bbe into master Oct 7, 2024
43 checks passed
@librelois librelois deleted the ahmad-evm-native-transfer-test branch October 7, 2024 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A8-mergeoncegreen Pull request is reviewed well. B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes D5-nicetohaveaudit⚠️ PR contains trivial changes to logic that should be properly reviewed. not-breaking Does not need to be mentioned in breaking changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants