-
Notifications
You must be signed in to change notification settings - Fork 335
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: batcher sends proof even if eip712 signature contents are incompatible #1005
base: staging
Are you sure you want to change the base?
fix: batcher sends proof even if eip712 signature contents are incompatible #1005
Conversation
@@ -4,7 +4,7 @@ version = "0.1.0" | |||
edition = "2021" | |||
|
|||
[dependencies] | |||
aligned-sdk = { git = "https://github.com/yetanotherco/aligned_layer", tag = "v0.6.0" } | |||
aligned-sdk = { git = "https://github.com/yetanotherco/aligned_layer", rev = "5ac0862ebace4e5971d1fd4c58f2c1973576d557" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we leave this version commit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The version should be the current one. Should I point it to the current branch? If i point it to an existing release, it won't contain the changes made in this PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes you are right. Let's make it point to the last commit of this branch, but we have to remember to update it once we make a release
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is it not using the repository version ? I assume we may want to keep them working while operators upgrade maybe ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The actual content of the requests does not change so the examples should be usable even before the SDK is released, right?
…2-signature-contents-are-incompatible
aside from some nits, code looks good and worked correctly |
This PR
There was a bug that the batcher accepted a proof if incompatible
--chain
and--batcher-payments-address
where provided, which resulted in an Ethereum tx revert for the whole batch submition.Resolving this, it was found that these 2 parameters where redundant, so they were both removed and replaced for the
--network
, which accepts eitherdevnet
,holesky-stage
orholesky
.Then, this parameter is used by the SDK to the appropriate contract addresses, while the chainId is resolved from the RPC (this has not changed, which also lead to confusion).
To Deploy
if client_msg.verification_data.payment_service_addr != self.payment_service.address() {
task sender
should be restarted using this new parameter (make targets have been migrated)To Test
You can test the new task senders, the batcher works correctly, and the examples