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: signer fills from if unset #555

Merged
merged 3 commits into from
Apr 17, 2024
Merged

fix: signer fills from if unset #555

merged 3 commits into from
Apr 17, 2024

Conversation

prestwich
Copy link
Member

Motivation

Unexpected behavior when both SignerFiller and NonceFiller are active causes nonces to not be filled properly, as from is unset. This kinda sucks

Solution

  • SignerFiller sets from if from is unset

PR Checklist

  • Added Tests
  • Added Documentation
  • Breaking changes

@prestwich prestwich added the enhancement New feature or request label Apr 17, 2024
@prestwich prestwich self-assigned this Apr 17, 2024
crates/provider/src/fillers/signer.rs Outdated Show resolved Hide resolved
Co-authored-by: Oliver Nordbjerg <onbjerg@users.noreply.github.com>
@prestwich prestwich merged commit 3faa1fc into main Apr 17, 2024
18 checks passed
@prestwich prestwich deleted the prestwich/signer-fill-from branch April 17, 2024 12:06
Comment on lines +49 to +52
if tx.from().is_none() {
return FillerControlFlow::Ready;
}

Copy link
Member

Choose a reason for hiding this comment

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

wait is this correct? shouldn't this be is_some(), "only skip signing by signaling ready if the from is set"

Copy link
Member Author

Choose a reason for hiding this comment

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

if it is none, we are ready for the signer to fill its default

ThreeHrSleep pushed a commit to ThreeHrSleep/alloy that referenced this pull request Apr 17, 2024
mattsse added a commit that referenced this pull request Apr 17, 2024
* Move BlockId type to alloy-eip

* suggested changes

* touchups

* chore: touchups

* fix: tmp fix for PendingTransactionBuilder::get_receipt (#558)

* feat: add prague engine types (#557)

* feat: add prague engine types

* Update crates/eips/src/eip6110.rs

Co-authored-by: DaniPopes <57450786+DaniPopes@users.noreply.github.com>

* clippy

* Update crates/eips/src/eip6110.rs

Co-authored-by: Oliver Nordbjerg <onbjerg@users.noreply.github.com>

* Update crates/rpc-types-engine/src/exit.rs

Co-authored-by: Oliver Nordbjerg <onbjerg@users.noreply.github.com>

---------

Co-authored-by: DaniPopes <57450786+DaniPopes@users.noreply.github.com>
Co-authored-by: Oliver Nordbjerg <onbjerg@users.noreply.github.com>

* fix: add more generics to any and receipt with bloom (#559)

* fix: signer fills from if unset (#555)

* fix: override txtype during submission prep (#556)

* Use the same way to both serialize and deserialize `OptimismPayloadAttributes::gas_limit`. (#563)

* Use the same way to both serialize and deserialize `OptimismPayloadAttributes::gas_limit`.

I understand that `op-reth` only ever deserializes this structure.
My use case is to serialize this struct and pass it to `op-geth`. The latter expects it to be hex.

* rustfmt

---------

Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de>

* feat: add ClientVersionV1 (#562)

* added ClientVersionV1

* touchups

* rustfmt

---------

Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de>

* clippy

* use alloc

---------

Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de>
Co-authored-by: Arsenii Kulikov <klkvrr@gmail.com>
Co-authored-by: DaniPopes <57450786+DaniPopes@users.noreply.github.com>
Co-authored-by: Oliver Nordbjerg <onbjerg@users.noreply.github.com>
Co-authored-by: James Prestwich <james@prestwi.ch>
Co-authored-by: Raman Hafiyatulin <r.gafiyatullin@me.com>
Co-authored-by: guha-rahul <52607971+guha-rahul@users.noreply.github.com>
ben186 pushed a commit to ben186/alloy that referenced this pull request Jul 27, 2024
ben186 pushed a commit to ben186/alloy that referenced this pull request Jul 27, 2024
* Move BlockId type to alloy-eip

* suggested changes

* touchups

* chore: touchups

* fix: tmp fix for PendingTransactionBuilder::get_receipt (alloy-rs#558)

* feat: add prague engine types (alloy-rs#557)

* feat: add prague engine types

* Update crates/eips/src/eip6110.rs

Co-authored-by: DaniPopes <57450786+DaniPopes@users.noreply.github.com>

* clippy

* Update crates/eips/src/eip6110.rs

Co-authored-by: Oliver Nordbjerg <onbjerg@users.noreply.github.com>

* Update crates/rpc-types-engine/src/exit.rs

Co-authored-by: Oliver Nordbjerg <onbjerg@users.noreply.github.com>

---------

Co-authored-by: DaniPopes <57450786+DaniPopes@users.noreply.github.com>
Co-authored-by: Oliver Nordbjerg <onbjerg@users.noreply.github.com>

* fix: add more generics to any and receipt with bloom (alloy-rs#559)

* fix: signer fills from if unset (alloy-rs#555)

* fix: override txtype during submission prep (alloy-rs#556)

* Use the same way to both serialize and deserialize `OptimismPayloadAttributes::gas_limit`. (alloy-rs#563)

* Use the same way to both serialize and deserialize `OptimismPayloadAttributes::gas_limit`.

I understand that `op-reth` only ever deserializes this structure.
My use case is to serialize this struct and pass it to `op-geth`. The latter expects it to be hex.

* rustfmt

---------

Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de>

* feat: add ClientVersionV1 (alloy-rs#562)

* added ClientVersionV1

* touchups

* rustfmt

---------

Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de>

* clippy

* use alloc

---------

Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de>
Co-authored-by: Arsenii Kulikov <klkvrr@gmail.com>
Co-authored-by: DaniPopes <57450786+DaniPopes@users.noreply.github.com>
Co-authored-by: Oliver Nordbjerg <onbjerg@users.noreply.github.com>
Co-authored-by: James Prestwich <james@prestwi.ch>
Co-authored-by: Raman Hafiyatulin <r.gafiyatullin@me.com>
Co-authored-by: guha-rahul <52607971+guha-rahul@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants