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

fix: batcher sends proof even if eip712 signature contents are incompatible #1005

Open
wants to merge 21 commits into
base: staging
Choose a base branch
from

Conversation

uri-99
Copy link
Contributor

@uri-99 uri-99 commented Sep 18, 2024

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 either devnet, holesky-stage or holesky .

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

  • As the SDK changes with this PR, a new release with a new tag should be made (maybe v0.7.1 or v0.8.0)
  • The Batcher should be redeployed, since a new check is being made
    • if client_msg.verification_data.payment_service_addr != self.payment_service.address() {
  • The 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

@uri-99 uri-99 self-assigned this Sep 18, 2024
@uri-99 uri-99 changed the title 999 fix batcher sends proof even if eip712 signature contents are incompatible fix: batcher sends proof even if eip712 signature contents are incompatible Sep 18, 2024
@uri-99 uri-99 marked this pull request as ready for review September 19, 2024 21:59
@@ -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" }
Copy link
Contributor

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?

Copy link
Contributor Author

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

Copy link
Contributor

@entropidelic entropidelic Sep 24, 2024

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

Copy link
Contributor

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 ?

Copy link
Contributor Author

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?

@entropidelic
Copy link
Contributor

aside from some nits, code looks good and worked correctly

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants