Skip to content

Commit

Permalink
Merge #2213: Fix InputWeightPrediction::P2WPKH_MAX constant DER sig l…
Browse files Browse the repository at this point in the history
…ength

f41ebc2 Add test for input weight predictions (conduition)
4514a80 Fix the InputWeightPrediction constants for DER signatures (conduition)
b5ce219 add weight method to InputWeightPrediction (conduition)

Pull request description:

  The `P2WPKH_MAX` constant assumed DER signatures in the witness have a max length of 73. In practice, their maximum length is 72, because [BIP62](https://github.com/bitcoin/bips/blob/master/bip-0062.mediawiki) forbids nodes from relaying transactions which contain non-canonical ECDSA signatures (i.e. TX sigs must have an $s$ value of less than $\frac{n}{2}$).

  This means $s$ is never encoded with a leading zero byte, and the signature as a whole never exceeds 72 bytes in total encoded length. The `ground_p2wpkh` function was already correct; only the constant needed to be corrected.

  Technically 73 bytes *is* the upper limit for signatures, as nothing forbids miners from including such non-standard transaction signatures in blocks, but for the purposes of fee estimation and input weight prediction, 72 is the number which 99.9% of implementations should use as their ceiling. We already use it as the ceiling for the `ground_p2wpkh` function - `ground_p2wpkh(0)` returns a prediction which uses a witness signature of length 72.

  Reference:
  - https://bitcoin.stackexchange.com/questions/77191/what-is-the-maximum-size-of-a-der-encoded-ecdsa-signature
  - https://bitcoin.stackexchange.com/questions/106435/are-high-s-ecdsa-signatures-forbidden-in-segwit-witnesses
  - https://github.com/bitcoin/bips/blob/master/bip-0062.mediawiki

  To enable testing, I added a `weight()` method to `InputWeightPrediction` and made it public but i'm not sure whether it has a use-case. Let me know if I should make it private instead.

ACKs for top commit:
  tcharding:
    ACK f41ebc2
  apoelstra:
    ACK f41ebc2

Tree-SHA512: 10e837bad9881c0efebb0598eaefd4ab039f2a6ececead75a68e253d84f5e85cb30496a6069eee8dfe9714773f3aa23cfe373f5d88d1c5609e1b1be1ff142e37
  • Loading branch information
apoelstra committed Nov 22, 2023
2 parents ba318f1 + f41ebc2 commit 2664f97
Showing 1 changed file with 59 additions and 3 deletions.
62 changes: 59 additions & 3 deletions bitcoin/src/blockdata/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1202,7 +1202,7 @@ where
|(count, partial_input_weight, inputs_with_witnesses), prediction| {
(
count + 1,
partial_input_weight + prediction.script_size * 4 + prediction.witness_size,
partial_input_weight + prediction.weight().to_wu() as usize,
inputs_with_witnesses + (prediction.witness_size > 0) as usize,
)
},
Expand Down Expand Up @@ -1276,7 +1276,7 @@ pub const fn predict_weight_from_slices(
let mut i = 0;
while i < inputs.len() {
let prediction = inputs[i];
partial_input_weight += prediction.script_size * 4 + prediction.witness_size;
partial_input_weight += prediction.weight().to_wu() as usize;
inputs_with_witnesses += (prediction.witness_size > 0) as usize;
i += 1;
}
Expand Down Expand Up @@ -1321,7 +1321,7 @@ impl InputWeightPrediction {
/// under-paying. See [`ground_p2wpkh`](Self::ground_p2wpkh) if you do use signature grinding.
///
/// [signature grinding]: https://bitcoin.stackexchange.com/questions/111660/what-is-signature-grinding
pub const P2WPKH_MAX: Self = InputWeightPrediction::from_slice(0, &[73, 33]);
pub const P2WPKH_MAX: Self = InputWeightPrediction::from_slice(0, &[72, 33]);

/// Input weight prediction corresponding to spending of a P2PKH output with the largest possible
/// DER-encoded signature, and a compressed public key.
Expand Down Expand Up @@ -1440,6 +1440,12 @@ impl InputWeightPrediction {

InputWeightPrediction { script_size, witness_size }
}

/// Tallies the total weight added to a transaction by an input with this weight prediction,
/// not counting potential witness flag bytes or the witness count varint.
pub const fn weight(&self) -> Weight {
Weight::from_wu_usize(self.script_size * 4 + self.witness_size)
}
}

#[cfg(test)]
Expand Down Expand Up @@ -2147,6 +2153,56 @@ mod tests {
assert_eq!(tx.total_sigop_cost(return_none), *expected_none);
}
}

#[test]
fn weight_predictions() {
// TXID 3d3381f968e3a73841cba5e73bf47dcea9f25a9f7663c51c81f1db8229a309a0
let tx_raw = hex!(
"01000000000103fc9aa70afba04da865f9821734b556cca9fb5710\
fc1338b97fba811033f755e308000000000000000019b37457784d\
d04936f011f733b8016c247a9ef08d40007a54a5159d1fc62ee216\
00000000000000004c4f2937c6ccf8256d9711a19df1ae62172297\
0bf46be925ff15f490efa1633d01000000000000000002c0e1e400\
0000000017a9146983f776902c1d1d0355ae0962cb7bc69e9afbde\
8706a1e600000000001600144257782711458506b89f255202d645\
e25c41144702483045022100dcada0499865a49d0aab8cb113c5f8\
3fd5a97abc793f97f3f53aa4b9d1192ed702202094c7934666a30d\
6adb1cc9e3b6bc14d2ffebd3200f3908c40053ef2df640b5012103\
15434bb59b615a383ae87316e784fc11835bb97fab33fdd2578025\
e9968d516e0247304402201d90b3197650569eba4bc0e0b1e2dca7\
7dfac7b80d4366f335b67e92e0546e4402203b4be1d443ad7e3a5e\
a92aafbcdc027bf9ccf5fe68c0bc8f3ebb6ab806c5464c012103e0\
0d92b0fe60731a54fdbcc6920934159db8ffd69d55564579b69a22\
ec5bb7530247304402205ab83b734df818e64d8b9e86a8a75f9d00\
5c0c6e1b988d045604853ab9ccbde002205a580235841df609d6bd\
67534bdcd301999b18e74e197e9e476cdef5fdcbf822012102ebb3\
e8a4638ede4721fb98e44e3a3cd61fecfe744461b85e0b6a6a1017\
5d5aca00000000"
);

let tx = Transaction::consensus_decode::<&[u8]>(&mut tx_raw.as_ref()).unwrap();
let input_weights = vec![
InputWeightPrediction::P2WPKH_MAX,
InputWeightPrediction::ground_p2wpkh(1),
InputWeightPrediction::ground_p2wpkh(1),
];
// Outputs: [P2SH, P2WPKH]

// Confirm the transaction's predicted weight matches its actual weight.
let predicted = predict_weight(input_weights, tx.script_pubkey_lens());
let expected = tx.weight();
assert_eq!(predicted, expected);

// Confirm signature grinding input weight predictions are aligned with constants.
assert_eq!(
InputWeightPrediction::ground_p2wpkh(0).weight(),
InputWeightPrediction::P2WPKH_MAX.weight()
);
assert_eq!(
InputWeightPrediction::ground_p2pkh_compressed(0).weight(),
InputWeightPrediction::P2PKH_COMPRESSED_MAX.weight()
);
}
}

#[cfg(bench)]
Expand Down

0 comments on commit 2664f97

Please sign in to comment.