Transaction signature leading zero bytes RLP encoding

[aidenbenner]

Version
ethers v0.4.0

Platform
WSL Ubuntu 4.19.128

Description
Hello I've been building and signing transactions with ether-rs and I'm intermittently getting this error when sending the txs to geth
rlp: non-canonical integer (leading zero bytes) for *big.Int, decoding into (types.DynamicFeeTx).R

I sign the transaction like this
​ ​tx.rlp_signed(1, &wallet.sign_transaction(&tx).await.unwrap());

From the eth wiki:
https://eth.wiki/fundamentals/rlp
"positive RLP integers must be represented in big endian binary form with no leading zeroes (thus making the integer value zero be equivalent to the empty byte array). Deserialised positive integers with leading zeroes must be treated as invalid."

Inspecting the code a little bit I see the rlp stream is constructed here

    /// Produces the RLP encoding of the transaction with the provided signature
    pub fn rlp_signed<T: Into<U64>>(&self, chain_id: T, signature: &Signature) -> Bytes {
        let mut rlp = RlpStream::new();
        rlp.begin_unbounded_list();
        let chain_id = chain_id.into();
        self.rlp_base(chain_id, &mut rlp);

        // append the signature
        let v = normalize_v(signature.v, chain_id);
        rlp.append(&v);
        rlp.append(&signature.r);
        rlp.append(&signature.s);
        rlp.finalize_unbounded_list();
        rlp.out().freeze().into()
    }

It seems that the leading 0's should be handled by the encoding if the r and s type in the signature were U256 but because they are H256 the leading 0's aren't removed.
https://github.com/paritytech/parity-common/blob/master/primitive-types/impls/rlp/src/lib.rs

I'd guess making this type change would fix the issue but I haven't tested yet (would love to know if there are other implications to changing this type)

[gakonst]

This assessment sounds correct to me, thank you for pointing this out and with detailed information!

The suggested fix should plausibly work without many side effects.

I won't be at my computer until eod tomorrow, but I'd be happy to merge a PR with a fix.

[gakonst]

@aidenbenner could you please check if #379 addresses the issue?

[aidenbenner]

LGTM - hard for me to test because it's been happening randomly but hasn't happened since thanks @roynalnaruto