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

Replace old packers #857

Merged
merged 26 commits into from
Dec 15, 2023
Merged

Replace old packers #857

merged 26 commits into from
Dec 15, 2023

Conversation

ceyonur
Copy link
Collaborator

@ceyonur ceyonur commented Sep 14, 2023

Why this should be merged

Initially we wrote our own Unpack/Pack methods and had a fixed length check for input bytes. In #487 we became aware that Solidity/JS libraries can add some extra padding bytes at the end of the input (input must still be multiple of 32).

This PR replaces our legacy Packers with ones from ABI package to provide a better support. Also we have removed
the input len check with the DUpgrade to support Solidity padding. But we still need to keep this check before DUpgrade for backwards compatibility.

Closes: #510 #487

Test branch #843

How this was tested

  • Unit tests
  • Local network testing

How is this documented

This makes the code on-par with recent Solidity docs.

@ceyonur ceyonur added this to the v0.5.6 milestone Sep 14, 2023
@ceyonur ceyonur self-assigned this Sep 14, 2023
darioush
darioush previously approved these changes Sep 14, 2023
Copy link
Collaborator

@darioush darioush left a comment

Choose a reason for hiding this comment

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

just some comments about the comments

precompile/contracts/feemanager/contract.go Outdated Show resolved Hide resolved
precompile/contracts/nativeminter/module.go Outdated Show resolved Hide resolved
precompile/contracts/feemanager/config.go Show resolved Hide resolved
ceyonur and others added 3 commits September 14, 2023 21:21
Co-authored-by: Darioush Jalali <darioush.jalali@avalabs.org>
Signed-off-by: Ceyhun Onur <ceyhunonur54@gmail.com>
@aaronbuchwald aaronbuchwald modified the milestones: v0.5.6, v0.5.7 Sep 19, 2023
@aaronbuchwald aaronbuchwald changed the base branch from abi-standardize to master October 4, 2023 21:45
@aaronbuchwald aaronbuchwald mentioned this pull request Oct 4, 2023
@ceyonur ceyonur removed this from the subnet-evm@Future Release milestone Oct 16, 2023
@ceyonur ceyonur linked an issue Nov 13, 2023 that may be closed by this pull request
@ceyonur ceyonur removed the request for review from anusha-ctrl November 28, 2023 15:55
@ceyonur ceyonur removed the Need Test label Dec 7, 2023
marun
marun previously approved these changes Dec 11, 2023
precompile/contracts/nativeminter/module.go Outdated Show resolved Hide resolved
precompile/contracts/deployerallowlist/module.go Outdated Show resolved Hide resolved
precompile/contracts/feemanager/module.go Show resolved Hide resolved
precompile/contracts/nativeminter/contract.go Show resolved Hide resolved
precompile/contracts/feemanager/unpack_pack_test.go Outdated Show resolved Hide resolved
precompile/contracts/feemanager/unpack_pack_test.go Outdated Show resolved Hide resolved
@ceyonur ceyonur requested a review from marun December 15, 2023 09:37
@ceyonur ceyonur enabled auto-merge (squash) December 15, 2023 18:06
@ceyonur ceyonur merged commit ccda76d into master Dec 15, 2023
8 checks passed
@ceyonur ceyonur deleted the replace-old-packers branch December 15, 2023 18:18
@ceyonur ceyonur restored the replace-old-packers branch December 15, 2023 18:58
@ceyonur ceyonur deleted the replace-old-packers branch December 15, 2023 18:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Status: In Progress 🏗
4 participants