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.

Sign up for GitHub

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

Jump to bottom

Add overloaded checkNSignatures #589

Merged
merged 4 commits into from
Aug 1, 2023
Merged

Add overloaded checkNSignatures #589

merged 4 commits into from
Aug 1, 2023

Conversation

akshay-ap
Copy link
Member

@akshay-ap akshay-ap commented Jun 14, 2023 •
edited by mmv08
Loading

This PR:

  • Changes the checkNSignatures method signature to accept an executor parameter
  • Migrates the legacy method to the CompatibiltiyFallbackHandler

Please pay attention to the test cases, we need to ensure that this thing is properly tested

@akshay-ap akshay-ap marked this pull request as draft June 14, 2023 09:55
@akshay-ap akshay-ap changed the title Try fitting checkNsigs in code size limit Try fitting overloaded checkNsigs in code size limit Jun 14, 2023
@akshay-ap akshay-ap force-pushed the test-checknsigs branch 2 times, most recently from 78d30b1 to 10eeeee Compare June 15, 2023 09:48
Base automatically changed from wip-eip-1271-fix to release/v1.5.0 June 30, 2023 13:29
@mmv08 mmv08 force-pushed the release/v1.5.0 branch 3 times, most recently from 1ba4f55 to 04403af Compare July 3, 2023 12:21
@mmv08
Copy link
Member

mmv08 commented Jul 3, 2023

Can this be closed?

@akshay-ap
Copy link
Member Author

Can this be closed?

yes

@akshay-ap akshay-ap closed this Jul 4, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Jul 4, 2023
@mmv08 mmv08 reopened this Jul 13, 2023
@mmv08 mmv08 changed the title Try fitting overloaded checkNsigs in code size limit Add overloaded checkNSignatures Jul 13, 2023
@mmv08 mmv08 requested review from a team, rmeissner, Uxio0 and mmv08 and removed request for a team July 18, 2023 09:00
@mmv08 mmv08 marked this pull request as ready for review July 18, 2023 09:00
akshay-ap
akshay-ap commented Aug 1, 2023
View reviewed changes
Copy link
Member Author

@akshay-ap akshay-ap left a comment

Choose a reason for hiding this comment

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

LGTM

@mmv08 mmv08 merged commit f9a316b into release/v1.5.0 Aug 1, 2023
11 checks passed
@mmv08 mmv08 deleted the test-checknsigs branch August 1, 2023 11:09
@mmv08 mmv08 added this to the Version 1.5.0 milestone Aug 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@mmv08 mmv08 mmv08 approved these changes

@rmeissner rmeissner Awaiting requested review from rmeissner rmeissner was automatically assigned from safe-global/safe-protocol

@Uxio0 Uxio0 Awaiting requested review from Uxio0 Uxio0 was automatically assigned from safe-global/safe-protocol

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Version 1.5.0
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@akshay-ap @mmv08