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

feat: implement sign_transaction on Trezor #100

Merged
merged 2 commits into from
Jan 6, 2024

Conversation

DaniPopes
Copy link
Member

@DaniPopes DaniPopes commented Jan 5, 2024

Motivation

Merge onto #83

Solution

PR Checklist

  • Added Tests
  • Added Documentation
  • Breaking changes

@DaniPopes DaniPopes mentioned this pull request Jan 5, 2024
3 tasks
Comment on lines +189 to +193
// TODO: Uncomment in 1.76
/*
let signature = if let Some(tx) = (tx as &dyn std::any::Any).downcast_ref::<TxEip1559>() {
*/
let signature = if let Some(tx) = tx.__downcast_ref::<TxEip1559>() {
Copy link
Member

Choose a reason for hiding this comment

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

good with me 👍

Comment on lines +10 to +17
/// A signable transaction.
pub type SignableTx = dyn Transaction<Signature = Signature>;

/// Extension trait for utilities for signable transactions.
///
/// This trait allows us to generically sign transactions without knowing the
/// receipt type, and is blanket implemented for transactions which our signer
/// trait can produce signatures for.
pub trait SignableTx: Send + Sync + 'static {
/// This trait is implemented for all types that implement [`Transaction`] with [`Signature`] as the
/// signature associated type.
pub trait TransactionExt: Transaction<Signature = Signature> {
Copy link
Member

Choose a reason for hiding this comment

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

good w me also

Comment on lines -17 to -19
/// The receipt type for this transaction.
type Receipt: Receipt;

Copy link
Member

Choose a reason for hiding this comment

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

we'll bring this back per Prestwich thinking right? in that case let's leave this commented out instead of removing

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't know honestly, should receipts be tied to a specific transaction? Keeping this will make it impossible to have signers use dyn transaction because know that also has to be specified


mod signed;
pub use signed::Signed;

/// Represents a minimal EVM transaction.
pub trait Transaction: Encodable + Decodable + Send + Sync + 'static {
pub trait Transaction: std::any::Any + Encodable + Send + Sync + 'static {
Copy link
Member

Choose a reason for hiding this comment

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

bleh do we need to explicitly declare the Any here? I guess it'll be always there for most types we impl transaction for, would add a note in the code on why it's needed

@DaniPopes DaniPopes merged commit 8a57c67 into prestwich/new-signature Jan 6, 2024
16 checks passed
@DaniPopes DaniPopes deleted the dani/trezor-signtx branch January 6, 2024 12:33
DaniPopes added a commit that referenced this pull request Jan 10, 2024
* refactor: signature keeps v r s in mem

wip: port ser tests

feature: Parity::chain_id

test: normalize 27/28

fix: properly normalize in recovery

fix: serde works now

refactor wip: lots of stuff

refactor: push signature into core primitives

feat: seal

feat: seal

fix: k256 feature in network

feature: rlp for TxLegacy

feature: block

wip test

fix: payload lenght

feat: sealable

feature: Tx types

tests: they pass????

refactor: envelope module

refactor: TxType

refactor: simpler rlp enc/decode for envelope

fix: todo

feature: enable sign_transaction

feat: some froms and some cleanup

lint: clippy

fix: test feature

fix: network receipt bounds

receipt.rs

restore providers

* fix: network

* fix: misc fixes for new signature methods

* feature: custom 2718 error

* fix: transaction may not be RLP

* remove log mod

* feature: Header, constants, and pure calculations

* refactor: impl Sealable

* refactor: move 2718 and add docs

* chore: enable lints

* fix: const fns

* fix: correct eip2718 network encoding behavior

* feature: arbitrary

* cleanup: deps and patch

* feature: Receipt trait

* fix: impl bloom_cheap

* feature: generic signature

* doc: document signature type

* fix: remove encodable bound

* refactor: signabletx trait

* fix: some lints and such

* fix: remove dbg

* fix: spacing

* feature: alloy-eips

* chore: port access list

* misc arbitrary cleanup

* fix: encode_2718 length

* fix: use core git main instead of local

* chore: clippy, docs

* fix: features

* fix: cap arbitrary logs

* chore: use Vec instead of BufMut where possible

* feat: fix Signer EIP-155, implement sign_transaction

* chore: make ledger internal sign tx public

* chore: remove eip1559 duplicate functions

* feat: implement sign_transaction on Trezor (#100)

* feat: implement sign_transaction on Trezor

* fix: dyn trait upcasting is not stable yet

* chore: relax requirements

* chore: address review

---------

Co-authored-by: DaniPopes <57450786+DaniPopes@users.noreply.github.com>
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.

2 participants