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

Update tx service delegate methods #427

Merged
merged 12 commits into from
Jul 4, 2024
Merged

Update tx service delegate methods #427

merged 12 commits into from
Jul 4, 2024

Conversation

falvaradorodriguez
Copy link
Contributor

Closes #425

@falvaradorodriguez falvaradorodriguez requested a review from a team as a code owner June 26, 2024 11:40
@falvaradorodriguez
Copy link
Contributor Author

A new release of safe-eth-py is needed as the method that calculates the delegate signature in the current version is incorrect. Anyway, the task work in safe-cli would be ready.

@coveralls
Copy link

Coverage Status

coverage: 87.308% (+0.03%) from 87.279%
when pulling 82ef773 on update_tx_service_delegate_methods
into a9427b2 on main.

@coveralls
Copy link

Coverage Status

coverage: 87.497% (+0.03%) from 87.47%
when pulling ca3750a on update_tx_service_delegate_methods
into ddd66d6 on main.

@coveralls
Copy link

Coverage Status

coverage: 88.593% (+0.02%) from 88.571%
when pulling 95e874c on update_tx_service_delegate_methods
into f343bb8 on main.

moisses89
moisses89 requested changes Jul 1, 2024
View reviewed changes
falvaradorodriguez and others added 3 commits July 1, 2024 13:17
@falvaradorodriguez @moisses89
Update var name
bf2fd69 
Co-authored-by: Moisés  <7888669+moisses89@users.noreply.github.com>
@falvaradorodriguez @moisses89
Update var name
d21ade9 
Co-authored-by: Moisés  <7888669+moisses89@users.noreply.github.com>
@falvaradorodriguez
Update test var names
465f46e
@coveralls
Copy link

Coverage Status

coverage: 88.602% (+0.03%) from 88.571%
when pulling 465f46e on update_tx_service_delegate_methods
into f343bb8 on main.

moisses89
moisses89 approved these changes Jul 3, 2024
View reviewed changes
tests/test_safe_tx_service_operator.py Outdated
expected_hash = safe_operator.safe_tx_service.create_delegate_message_hash(
delegate_address
)
expected_signature_signed = signer.signHash(expected_hash)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
expected_signature_signed = signer.signHash(expected_hash)
expected_signature = signer.signHash(expected_hash)

Looks like a bit redundant signature is supposed to be signed :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, you are right. Updated b8f86ad

@falvaradorodriguez
Copy link
Contributor Author

Added a blocked label to wait for a new version of safe-eth-py with the signature EIP712 fixed

@coveralls
Copy link

Coverage Status

coverage: 88.602% (+0.02%) from 88.58%
when pulling ab20b37 on update_tx_service_delegate_methods
into d59dc92 on main.

@moisses89 moisses89 mentioned this pull request Jul 3, 2024
Closed
@coveralls
Copy link

Coverage Status

coverage: 88.602% (+0.02%) from 88.58%
when pulling 58f2ece on update_tx_service_delegate_methods
into d59dc92 on main.

@falvaradorodriguez falvaradorodriguez merged commit 8303420 into main Jul 4, 2024
13 checks passed
@falvaradorodriguez falvaradorodriguez deleted the update_tx_service_delegate_methods branch July 4, 2024 11:39
@github-actions github-actions bot locked and limited conversation to collaborators Jul 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@moisses89 moisses89 moisses89 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Update transaction service delegate methods
3 participants
@falvaradorodriguez @coveralls @moisses89