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

Feature: bytecode size reduction by removing public getChainId function, making encodeTransactionData private #603

Merged
merged 1 commit into from
Jul 10, 2023

Conversation

mmv08
Copy link
Member

@mmv08 mmv08 commented Jul 6, 2023

This PR is a pre-requisite for future 1.5.0 features like the overloaded checkNSignatures and the module guard.

Code size after all the changes:

Safe 22913 bytes (limit is 24576)
SafeL2 23755 bytes (limit is 24576)

@mmv08 mmv08 requested review from a team, rmeissner, Uxio0 and akshay-ap and removed request for a team July 6, 2023 09:31
@@ -58,13 +58,6 @@ describe("Safe", async () => {
});
});

describe("getChainId", async () => {
Copy link
Member Author

Choose a reason for hiding this comment

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

This should be still covered by the domain separator test

@github-actions
Copy link

github-actions bot commented Jul 6, 2023

Pull Request Test Coverage Report for Build 5473862599

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.01%) to 93.976%

Totals Coverage Status
Change from base Build 5455854105: -0.01%
Covered Lines: 313
Relevant Lines: 328

💛 - Coveralls

@mmv08 mmv08 merged commit db34e78 into release/v1.5.0 Jul 10, 2023
@mmv08 mmv08 deleted the feature/bytecode-shaving-2 branch July 10, 2023 11:24
@github-actions github-actions bot locked and limited conversation to collaborators Jul 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants