Skip to content

refactor: remove 0 check in recovered address in BatcherPaymentService - #1064

Merged
MauroToscano merged 2 commits into
stagingfrom
1061-remove-0-address-check-from-recovered-signer
Sep 25, 2024
Merged

refactor: remove 0 check in recovered address in BatcherPaymentService#1064
MauroToscano merged 2 commits into
stagingfrom
1061-remove-0-address-check-from-recovered-signer

Conversation

@MarcosNicolau

@MarcosNicolauMarcosNicolau commented Sep 24, 2024

Copy link
Copy Markdown
Collaborator

Changes
This PR removes the check for zero addresses in the recovered signature within the BatcherPaymentService contract. The library handling the signature recovery already performs this check when splitting the fields (r, v, s) first and then calling the ecrecover precompile, where it does the check (see here).

Testing
I've tested that everything works by setting up a local environment and sending some sp1 and plonk proofs. Here is the process:

  1. Start aggregator: make aggregator_start.
  2. Start operator: make operator_register_and_start.
  3. Start batcher: make batcher_start_local.
  4. Send some proofs and make sure everything works as expected.

Contracts have been updated with make anvil_deploy_aligned_contracts.

Closes#1061

@MarcosNicolauMarcosNicolau changed the title refactor: remove check in BatcherPaymentServicerefactor: remove 0 check in recovered address in BatcherPaymentServiceSep 24, 2024
@github-actions

github-actionsBot commented Sep 24, 2024

Copy link
Copy Markdown

Changes to gas cost

Generated at commit: d8d846c25009e143caebf4976dc84642e84b428e, compared to commit: 4b2fabe8efd54cc36a600f08767aaaeec90cd991

🧾 Summary (10% most significant diffs)

ContractMethodAvg (+/-)%
AlignedLayerServiceManagercreateNewTask+318 ❌+0.43%

Full diff report 👇
ContractDeployment Cost (+/-)MethodMin (+/-)%Avg (+/-)%Median (+/-)%Max (+/-)%# Calls (+/-)
AlignedLayerServiceManager4,629,415 (0)createNewTask
receive
53,907 (+84)
21,169 (0)
+0.16%
0.00%
73,971 (+318)
44,970 (+373)
+0.43%
+0.84%
73,975 (+6)
45,064 (0)
+0.01%
0.00%
74,746 (0)
45,064 (0)
0.00%
0.00%
256 (0)
256 (0)

@MarcosNicolauMarcosNicolau self-assigned this Sep 24, 2024

@MauroToscanoMauroToscano left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Re-deploy aligned contracts and test it, and push them

@MauroToscano
MauroToscano merged commit c9518d7 into stagingSep 25, 2024
@MauroToscano
MauroToscano deleted the 1061-remove-0-address-check-from-recovered-signer branch September 25, 2024 15:57
This was referenced Sep 30, 2024
Sign up for freeto 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.

4 participants

@MarcosNicolau@MauroToscano@uri-99@entropidelic