diff --git a/chain-signatures/node/src/sign_bidirectional.rs b/chain-signatures/node/src/sign_bidirectional.rs index 689883d9..076563ae 100644 --- a/chain-signatures/node/src/sign_bidirectional.rs +++ b/chain-signatures/node/src/sign_bidirectional.rs @@ -260,12 +260,17 @@ pub fn sign_and_hash_transaction( unsigned_rlp: &[u8], signature: Signature, ) -> anyhow::Result<([u8; 32], u64)> { - let r = signature.big_r.x().as_slice().to_vec(); - let s = signature.s.to_bytes().as_slice().to_vec(); + let r_bytes = signature.big_r.x(); + let s_bytes = signature.s.to_bytes(); let y_parity = signature.recovery_id == 1; if is_eip1559(unsigned_rlp) { - sign_and_hash_eip1559_from_unsigned(unsigned_rlp, &r, &s, y_parity) + sign_and_hash_eip1559_from_unsigned( + unsigned_rlp, + r_bytes.as_slice(), + s_bytes.as_slice(), + y_parity, + ) } else { // Extract chain_id from the unsigned RLP (it's the 7th field in legacy transactions) // In legacy Ethereum transactions with EIP-155, there are 9 fields: @@ -278,7 +283,13 @@ pub fn sign_and_hash_transaction( } else { None }; - sign_and_hash_legacy_from_unsigned(unsigned_rlp, chain_id, &r, &s, y_parity) + sign_and_hash_legacy_from_unsigned( + unsigned_rlp, + chain_id, + r_bytes.as_slice(), + s_bytes.as_slice(), + y_parity, + ) } } @@ -286,6 +297,12 @@ fn is_eip1559(unsigned_rlp: &[u8]) -> bool { unsigned_rlp[0] == 0x02 } +fn trim_rlp_leading_zeroes(bytes: &[u8]) -> Vec { + // RLP integers must be minimally encoded; drop redundant leading zero octets. + let first_non_zero = bytes.iter().position(|b| *b != 0).unwrap_or(bytes.len()); + bytes[first_non_zero..].to_vec() +} + pub fn sign_and_hash_eip1559_from_unsigned( unsigned: &[u8], // may be 0x02||RLP(body) or just RLP(body) r: &[u8], @@ -315,6 +332,8 @@ pub fn sign_and_hash_eip1559_from_unsigned( } let y: u8 = if y_parity { 1 } else { 0 }; srlp.append(&y); + let r = trim_rlp_leading_zeroes(r); + let s = trim_rlp_leading_zeroes(s); srlp.append(&r); srlp.append(&s); @@ -348,6 +367,8 @@ pub fn sign_and_hash_legacy_from_unsigned( } let v: u64 = 35 + 2 * chain_id.unwrap_or(0) + if y_parity { 1 } else { 0 }; out.append(&v); + let r = trim_rlp_leading_zeroes(r); + let s = trim_rlp_leading_zeroes(s); out.append(&r); out.append(&s); @@ -356,6 +377,29 @@ pub fn sign_and_hash_legacy_from_unsigned( Ok((hash.into(), nonce)) } +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn trim_rlp_bytes_drops_leading_zeroes() { + let bytes = [0u8, 0x00, 0x01, 0xAB]; + assert_eq!(trim_rlp_leading_zeroes(&bytes), vec![0x01, 0xAB]); + } + + #[test] + fn trim_rlp_bytes_all_zeroes_becomes_empty() { + let bytes = [0u8; 4]; + assert!(trim_rlp_leading_zeroes(&bytes).is_empty()); + } + + #[test] + fn trim_rlp_bytes_leaves_minimal_value() { + let bytes = [0x7Fu8, 0x10]; + assert_eq!(trim_rlp_leading_zeroes(&bytes), bytes.to_vec()); + } +} + /// Get the x coordinate of a point, as a scalar fn x_coordinate(point: &C::AffinePoint) -> C::Scalar { ::Uint>>::reduce_bytes(&point.x())