Skip to content

feat: use alloy Signature type#10758

Merged
emhane merged 31 commits intoparadigmxyz:mainfrom
leruaa:use_alloy_signature_type
Sep 23, 2024
Merged

feat: use alloy Signature type#10758
emhane merged 31 commits intoparadigmxyz:mainfrom
leruaa:use_alloy_signature_type

Conversation

@leruaa
Copy link
Copy Markdown
Contributor

@leruaa leruaa commented Sep 6, 2024

Closes #10750

@leruaa leruaa marked this pull request as ready for review September 15, 2024 21:35
Copy link
Copy Markdown
Collaborator

@emhane emhane left a comment

Choose a reason for hiding this comment

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

lgtm

Comment thread crates/optimism/evm/src/execute.rs
Copy link
Copy Markdown
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

cool!

@klkvr could you please take a closer look here

@mattsse mattsse added the C-debt A clean up/refactor of existing code label Sep 17, 2024
}
let signature_hash = self.signature_hash();
self.signature.recover_signer(signature_hash)
recover_signer(&self.signature, signature_hash)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

should we use signature.recover here? @mattsse

@mattsse
Copy link
Copy Markdown
Collaborator

mattsse commented Sep 20, 2024

cool, once ready I can do a new alloy patch so we have the parity checks

@leruaa leruaa requested a review from onbjerg as a code owner September 21, 2024 15:57
@emhane emhane requested review from klkvr and mattsse September 21, 2024 17:15
@emhane
Copy link
Copy Markdown
Collaborator

emhane commented Sep 21, 2024

let's get this beefy pr in before more merge conflicts arise @klkvr @mattsse

Copy link
Copy Markdown
Member

@klkvr klkvr left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Copy Markdown
Collaborator

@onbjerg onbjerg left a comment

Choose a reason for hiding this comment

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

lgtm, but we should take care to test this on an existing db (as with other prim type changes) before next release to ensure we didn't break the db

@emhane
Copy link
Copy Markdown
Collaborator

emhane commented Sep 23, 2024

lgtm, but we should take care to test this on an existing db (as with other prim type changes) before next release to ensure we didn't break the db

you mean sync a node from scratch? can do

@emhane emhane added this pull request to the merge queue Sep 23, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to a conflict with the base branch Sep 23, 2024
@emhane emhane enabled auto-merge September 23, 2024 13:17
@emhane emhane added this pull request to the merge queue Sep 23, 2024
Merged via the queue into paradigmxyz:main with commit 15aee9b Sep 23, 2024
theochap pushed a commit to ethereum-optimism/optimism that referenced this pull request Jan 22, 2026
Co-authored-by: Emilia Hane <elsaemiliaevahane@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-debt A clean up/refactor of existing code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

use alloy Signature type

5 participants