[Bug] Signers apply EIP-155 even for EIP-{1559,2718} transactions

[cgst]

Component

signers

What version of Alloy are you on?

No response

Operating System

None

Describe the bug

Signers::sign_transaction and SignerSync::sign_transaction_sync should not apply EIP-155 when the signer's chain_id is None. Indeed, this is the expected behavior according to Signer's docstring, but EIP-155 is applied anyway if the chain ID is found on the transaction instead.

Ref:

alloy/crates/signer/src/signer.rs

Line 87 in 2ee7b8b

if let Some(chain_id) = chain_id.or_else(|| tx.chain_id()) {

EIP-{1559,2718} always embed a chain ID value but they're not compatible with EIP-155. Thus, they get an EIP-155 signature when they shouldn't.

[cgst]

I can send a PR for a direct fix if useful, but this comment made me think that there are more changes coming that might preempt a direct fix.

[prestwich]

the "correct" fix here is to change the Transaction::into_signed implementation to ensure that the signature parity is in the correct format when instantiating a Signed<T>

[cgst]

the "correct" fix here is to change the Transaction::into_signed implementation to ensure that the signature parity is in the correct format when instantiating a Signed<T>

🤔 Transaction::into_signed takes a Signature, so EIP-155 will have already been applied by that time. Are you saying we should "undo" the EIP-155 application?

I'm not 100% sure, but from looking at the code, if s is a y-parity Signature, then s.with_chain_id(...).to_parity_bool() != s, i.e., the roundtrip won't preserve the y-parity value. I'm not a cryptographer though, so maybe there's an easy way to do it.

Perhaps we should avoid applying EIP-155 in the first place. For example, the Transaction trait could indicate compatibility with EIP-155, so the signer has all the information it needs to decide what signature is required.

[prestwich]

Transaction::into_signed takes a Signature, so EIP-155 will have already been applied by that time. Are you saying we should "undo" the EIP-155 application?

I'm saying that the txn should ensure the signature is in the format that it accepts at the time it is joined with the siganture.
The problem is that a signature may come from any source with or without eip155 applied. We can guarantee correct format at the time of combination, but not necessarily at any other time.

I'm not 100% sure, but from looking at the code, if s is a y-parity Signature, then s.with_chain_id(...).to_parity_bool() != s, i.e., the roundtrip won't preserve the y-parity value. I'm not a cryptographer though, so maybe there's an easy way to do it.

That's a bug, thanks. fixed in 497