Skip to content

Commit d7bdf28

Browse files
authored
Refactor transaction signature validation (#75)
* Refactor transaction signature validation * fix clippy warning * fix remaining clippy warnings
1 parent cf3076f commit d7bdf28

9 files changed

Lines changed: 153 additions & 52 deletions

File tree

src/block.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ impl<T: EnvelopedDecodable> rlp::Decodable for Block<T> {
5858
impl<T: EnvelopedEncodable> Block<T> {
5959
pub fn new(partial_header: PartialHeader, transactions: Vec<T>, ommers: Vec<Header>) -> Self {
6060
let ommers_hash =
61-
H256::from_slice(Keccak256::digest(&rlp::encode_list(&ommers)[..]).as_slice());
61+
H256::from_slice(Keccak256::digest(&rlp::encode_list(&ommers)[..]).as_ref());
6262
let transactions_root = ordered_trie_root(
6363
transactions
6464
.iter()

src/header.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ impl Header {
5353

5454
#[must_use]
5555
pub fn hash(&self) -> H256 {
56-
H256::from_slice(Keccak256::digest(rlp::encode(self)).as_slice())
56+
H256::from_slice(Keccak256::digest(rlp::encode(self)).as_ref())
5757
}
5858
}
5959

src/transaction/eip1559.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ impl EIP1559Transaction {
3636
let mut out = alloc::vec![0; 1 + encoded.len()];
3737
out[0] = 2;
3838
out[1..].copy_from_slice(&encoded);
39-
H256::from_slice(Keccak256::digest(&out).as_slice())
39+
H256::from_slice(Keccak256::digest(&out).as_ref())
4040
}
4141

4242
pub fn to_message(self) -> EIP1559TransactionMessage {
@@ -118,7 +118,7 @@ impl EIP1559TransactionMessage {
118118
let mut out = alloc::vec![0; 1 + encoded.len()];
119119
out[0] = 2;
120120
out[1..].copy_from_slice(&encoded);
121-
H256::from_slice(Keccak256::digest(&out).as_slice())
121+
H256::from_slice(Keccak256::digest(&out).as_ref())
122122
}
123123
}
124124

src/transaction/eip2930.rs

Lines changed: 6 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ use ethereum_types::{Address, H256, U256};
44
use rlp::{DecoderError, Rlp, RlpStream};
55
use sha3::{Digest, Keccak256};
66

7+
use super::signature;
78
use crate::Bytes;
89

910
pub use super::legacy::TransactionAction;
@@ -44,18 +45,8 @@ pub struct TransactionSignature {
4445
impl TransactionSignature {
4546
#[must_use]
4647
pub fn new(odd_y_parity: bool, r: H256, s: H256) -> Option<Self> {
47-
const LOWER: H256 = H256([
48-
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
49-
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
50-
0x00, 0x00, 0x00, 0x01,
51-
]);
52-
const UPPER: H256 = H256([
53-
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
54-
0xff, 0xfe, 0xba, 0xae, 0xdc, 0xe6, 0xaf, 0x48, 0xa0, 0x3b, 0xbf, 0xd2, 0x5e, 0x8c,
55-
0xd0, 0x36, 0x41, 0x41,
56-
]);
57-
58-
let is_valid = r < UPPER && r >= LOWER && s < UPPER && s >= LOWER;
48+
let is_valid = signature::is_valid_signature_component(&r)
49+
&& signature::is_valid_signature_component(&s);
5950

6051
if is_valid {
6152
Some(Self { odd_y_parity, r, s })
@@ -81,13 +72,7 @@ impl TransactionSignature {
8172

8273
#[must_use]
8374
pub fn is_low_s(&self) -> bool {
84-
const LOWER: H256 = H256([
85-
0x7f, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
86-
0xff, 0xff, 0x5d, 0x57, 0x6e, 0x73, 0x57, 0xa4, 0x50, 0x1d, 0xdf, 0xe9, 0x2f, 0x46,
87-
0x68, 0x1b, 0x20, 0xa0,
88-
]);
89-
90-
self.s <= LOWER
75+
signature::is_low_s(&self.s)
9176
}
9277
}
9378

@@ -180,7 +165,7 @@ impl EIP2930Transaction {
180165
let mut out = alloc::vec![0; 1 + encoded.len()];
181166
out[0] = 1;
182167
out[1..].copy_from_slice(&encoded);
183-
H256::from_slice(Keccak256::digest(&out).as_slice())
168+
H256::from_slice(Keccak256::digest(&out).as_ref())
184169
}
185170

186171
pub fn to_message(self) -> EIP2930TransactionMessage {
@@ -258,7 +243,7 @@ impl EIP2930TransactionMessage {
258243
let mut out = alloc::vec![0; 1 + encoded.len()];
259244
out[0] = 1;
260245
out[1..].copy_from_slice(&encoded);
261-
H256::from_slice(Keccak256::digest(&out).as_slice())
246+
H256::from_slice(Keccak256::digest(&out).as_ref())
262247
}
263248
}
264249

src/transaction/eip7702.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ impl AuthorizationListItem {
139139
message.extend_from_slice(&rlp_stream.out());
140140

141141
// Return keccak256 hash of the complete message
142-
H256::from_slice(Keccak256::digest(&message).as_slice())
142+
H256::from_slice(Keccak256::digest(&message).as_ref())
143143
}
144144

145145
/// Convert VerifyingKey to Ethereum address
@@ -196,7 +196,7 @@ impl EIP7702Transaction {
196196
let mut out = alloc::vec![0; 1 + encoded.len()];
197197
out[0] = SET_CODE_TX_TYPE;
198198
out[1..].copy_from_slice(&encoded);
199-
H256::from_slice(Keccak256::digest(&out).as_slice())
199+
H256::from_slice(Keccak256::digest(&out).as_ref())
200200
}
201201

202202
pub fn to_message(self) -> EIP7702TransactionMessage {
@@ -282,7 +282,7 @@ impl EIP7702TransactionMessage {
282282
let mut out = alloc::vec![0; 1 + encoded.len()];
283283
out[0] = SET_CODE_TX_TYPE;
284284
out[1..].copy_from_slice(&encoded);
285-
H256::from_slice(Keccak256::digest(&out).as_slice())
285+
H256::from_slice(Keccak256::digest(&out).as_ref())
286286
}
287287
}
288288

@@ -342,7 +342,7 @@ mod tests {
342342
rlp_stream.append(&address);
343343
rlp_stream.append(&nonce);
344344
message.extend_from_slice(&rlp_stream.out());
345-
let message_hash = H256::from_slice(Keccak256::digest(&message).as_slice());
345+
let message_hash = H256::from_slice(Keccak256::digest(&message).as_ref());
346346

347347
// Sign the message hash
348348
let (signature, recovery_id) = signing_key

src/transaction/legacy.rs

Lines changed: 7 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ use ethereum_types::{H160, H256, U256};
44
use rlp::{DecoderError, Rlp, RlpStream};
55
use sha3::{Digest, Keccak256};
66

7+
use super::signature;
78
use crate::Bytes;
89

910
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
@@ -101,19 +102,10 @@ pub struct TransactionSignature {
101102
impl TransactionSignature {
102103
#[must_use]
103104
pub fn new(v: u64, r: H256, s: H256) -> Option<Self> {
104-
const LOWER: H256 = H256([
105-
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
106-
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
107-
0x00, 0x00, 0x00, 0x01,
108-
]);
109-
const UPPER: H256 = H256([
110-
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
111-
0xff, 0xfe, 0xba, 0xae, 0xdc, 0xe6, 0xaf, 0x48, 0xa0, 0x3b, 0xbf, 0xd2, 0x5e, 0x8c,
112-
0xd0, 0x36, 0x41, 0x41,
113-
]);
114-
115105
let v = TransactionRecoveryId(v);
116-
let is_valid = v.standard() <= 1 && r < UPPER && r >= LOWER && s < UPPER && s >= LOWER;
106+
let is_valid = v.standard() <= 1
107+
&& signature::is_valid_signature_component(&r)
108+
&& signature::is_valid_signature_component(&s);
117109

118110
if is_valid {
119111
Some(Self { v, r, s })
@@ -149,13 +141,7 @@ impl TransactionSignature {
149141

150142
#[must_use]
151143
pub fn is_low_s(&self) -> bool {
152-
const LOWER: H256 = H256([
153-
0x7f, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
154-
0xff, 0xff, 0x5d, 0x57, 0x6e, 0x73, 0x57, 0xa4, 0x50, 0x1d, 0xdf, 0xe9, 0x2f, 0x46,
155-
0x68, 0x1b, 0x20, 0xa0,
156-
]);
157-
158-
self.s <= LOWER
144+
signature::is_low_s(&self.s)
159145
}
160146
}
161147

@@ -227,7 +213,7 @@ pub struct LegacyTransaction {
227213

228214
impl LegacyTransaction {
229215
pub fn hash(&self) -> H256 {
230-
H256::from_slice(Keccak256::digest(rlp::encode(self)).as_slice())
216+
H256::from_slice(Keccak256::digest(rlp::encode(self)).as_ref())
231217
}
232218

233219
pub fn to_message(self) -> LegacyTransactionMessage {
@@ -295,7 +281,7 @@ pub struct LegacyTransactionMessage {
295281

296282
impl LegacyTransactionMessage {
297283
pub fn hash(&self) -> H256 {
298-
H256::from_slice(Keccak256::digest(rlp::encode(self)).as_slice())
284+
H256::from_slice(Keccak256::digest(rlp::encode(self)).as_ref())
299285
}
300286
}
301287

src/transaction/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ pub mod eip1559;
22
pub mod eip2930;
33
pub mod eip7702;
44
pub mod legacy;
5+
mod signature;
56

67
use bytes::BytesMut;
78
use ethereum_types::H256;

src/transaction/signature.rs

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
use ethereum_types::H256;
2+
3+
// ECDSA signature validation constants for secp256k1 curve
4+
5+
/// Minimum valid value for signature components r and s (must be >= 1)
6+
pub const SIGNATURE_LOWER_BOUND: H256 = H256([
7+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
8+
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01,
9+
]);
10+
11+
/// Maximum valid value for signature components r and s (must be < secp256k1 curve order)
12+
/// This is the secp256k1 curve order: 0xfffffffffffffffffffffffffffffffebaaedce6af48a03bbfd25e8cd0364141
13+
pub const SIGNATURE_UPPER_BOUND: H256 = H256([
14+
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xfe,
15+
0xba, 0xae, 0xdc, 0xe6, 0xaf, 0x48, 0xa0, 0x3b, 0xbf, 0xd2, 0x5e, 0x8c, 0xd0, 0x36, 0x41, 0x41,
16+
]);
17+
18+
/// Maximum value for low-s signature enforcement (half of curve order)
19+
/// This is used to prevent signature malleability
20+
/// Value: 0x7fffffffffffffffffffffffffffffff5d576e7357a4501ddfe92f46681b20a0
21+
pub const SIGNATURE_LOW_S_BOUND: H256 = H256([
22+
0x7f, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
23+
0x5d, 0x57, 0x6e, 0x73, 0x57, 0xa4, 0x50, 0x1d, 0xdf, 0xe9, 0x2f, 0x46, 0x68, 0x1b, 0x20, 0xa0,
24+
]);
25+
26+
/// Validates that a signature component (r or s) is within valid range
27+
///
28+
/// A valid signature component must satisfy:
29+
/// - Greater than or equal to 1 (SIGNATURE_LOWER_BOUND)
30+
/// - Less than the secp256k1 curve order (SIGNATURE_UPPER_BOUND)
31+
#[inline]
32+
pub fn is_valid_signature_component(component: &H256) -> bool {
33+
*component >= SIGNATURE_LOWER_BOUND && *component < SIGNATURE_UPPER_BOUND
34+
}
35+
36+
/// Checks if the s component satisfies the low-s requirement
37+
///
38+
/// The low-s requirement helps prevent signature malleability by requiring
39+
/// that s <= n/2 where n is the curve order.
40+
#[inline]
41+
pub fn is_low_s(s: &H256) -> bool {
42+
*s <= SIGNATURE_LOW_S_BOUND
43+
}
44+
45+
#[cfg(test)]
46+
mod tests {
47+
use super::*;
48+
use ethereum_types::U256;
49+
50+
/// Helper function to convert H256 to U256 for arithmetic operations in tests
51+
#[inline]
52+
fn h256_to_u256(h: &H256) -> U256 {
53+
U256::from_big_endian(h.as_bytes())
54+
}
55+
56+
/// Helper function to convert U256 to H256
57+
#[inline]
58+
fn u256_to_h256(u: U256) -> H256 {
59+
H256::from(u.to_big_endian())
60+
}
61+
62+
#[test]
63+
fn test_low_s_bound_is_half_curve_order() {
64+
// SIGNATURE_LOW_S_BOUND should be exactly n/2 where n is the curve order
65+
let n = h256_to_u256(&SIGNATURE_UPPER_BOUND);
66+
let expected_half_n = u256_to_h256(n / 2);
67+
68+
assert_eq!(
69+
SIGNATURE_LOW_S_BOUND, expected_half_n,
70+
"SIGNATURE_LOW_S_BOUND must be exactly half of the curve order"
71+
);
72+
}
73+
74+
#[test]
75+
fn test_signature_bounds() {
76+
// Lower bound is 1
77+
assert_eq!(SIGNATURE_LOWER_BOUND, H256::from_low_u64_be(1));
78+
79+
// Verify that 0 is invalid
80+
assert!(!is_valid_signature_component(&H256::zero()));
81+
82+
// Verify that 1 is valid (minimum)
83+
assert!(is_valid_signature_component(&H256::from_low_u64_be(1)));
84+
85+
// Verify that curve_order - 1 is valid (maximum)
86+
let max_valid = u256_to_h256(h256_to_u256(&SIGNATURE_UPPER_BOUND) - 1);
87+
assert!(is_valid_signature_component(&max_valid));
88+
89+
// Verify that curve_order itself is invalid
90+
assert!(!is_valid_signature_component(&SIGNATURE_UPPER_BOUND));
91+
92+
// Verify that values above curve_order are invalid
93+
let above_max = u256_to_h256(h256_to_u256(&SIGNATURE_UPPER_BOUND) + 1);
94+
assert!(!is_valid_signature_component(&above_max));
95+
}
96+
97+
#[test]
98+
fn test_low_s_validation() {
99+
// s = 0 is invalid (below lower bound)
100+
assert!(!is_valid_signature_component(&H256::zero()));
101+
102+
// s = 1 satisfies low-s requirement
103+
assert!(is_low_s(&u256_to_h256(U256::one())));
104+
105+
// s = low_s_bound satisfies low-s requirement (boundary)
106+
assert!(is_low_s(&SIGNATURE_LOW_S_BOUND));
107+
108+
// s = low_s_bound + 1 does NOT satisfy low-s requirement
109+
let above_low_s = u256_to_h256(h256_to_u256(&SIGNATURE_LOW_S_BOUND) + 1);
110+
assert!(!is_low_s(&above_low_s));
111+
112+
// s = curve_order - 1 is valid but does NOT satisfy low-s
113+
let high_s = u256_to_h256(h256_to_u256(&SIGNATURE_UPPER_BOUND) - 1);
114+
assert!(is_valid_signature_component(&high_s));
115+
assert!(!is_low_s(&high_s));
116+
}
117+
118+
#[test]
119+
fn test_boundary_conditions() {
120+
// Test exact boundary values
121+
assert_eq!(h256_to_u256(&SIGNATURE_LOWER_BOUND), U256::one());
122+
123+
// Ensure low-s bound is exactly half the curve order (curve_order / 2)
124+
// Note: The curve order is odd, so half_order * 2 + 1 = curve_order
125+
let curve_order = h256_to_u256(&SIGNATURE_UPPER_BOUND);
126+
let half_order = h256_to_u256(&SIGNATURE_LOW_S_BOUND);
127+
assert_eq!(curve_order / 2, half_order);
128+
}
129+
}

src/util.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ impl Hasher for KeccakHasher {
1717
const LENGTH: usize = 32;
1818

1919
fn hash(x: &[u8]) -> Self::Out {
20-
H256::from_slice(Keccak256::digest(x).as_slice())
20+
H256::from_slice(Keccak256::digest(x).as_ref())
2121
}
2222
}
2323

@@ -178,7 +178,7 @@ mod tests {
178178
const LENGTH: usize = 32;
179179

180180
fn hash(x: &[u8]) -> Self::Out {
181-
H256::from_slice(Keccak256::digest(x).as_slice())
181+
H256::from_slice(Keccak256::digest(x).as_ref())
182182
}
183183
}
184184

0 commit comments

Comments
 (0)