docs: clarify BIP34 rules and fix related test
What the developer wrote
Authored by jrakibi
docs: clarify BIP34 rules and fix related test
According to BIP34, the coinbase scriptSig must start with a push of
the block height, which already supersedes the lower bound of 2 bytes.
We keep EMPTY_COINBASE producing a 0-byte scriptSig for backward
compatibility with PSBT. Add a comment explaining this behavior.
Also update a test failing due to the new validation rule
What changed, and why it matters
This commit is a documentation and test fix, not a security patch. It adds a comment explaining that an 'empty coinbase' helper produces a 0-byte scriptSig that is technically invalid under Bitcoin consensus rules (which require 2-100 bytes), but is kept for backward compatibility in PSBT workflows where the scriptSig is filled in later. It also re-enables and updates a previously ignored unit test to use a valid 2-byte coinbase scriptSig.
No security action required. Reviewers may consider whether the EMPTY_COINBASE helper's invalid-by-default state could be misused, but the commit explicitly documents this and it is not a new issue.
Security signals we found
Documentation of consensus-invalid helper (EMPTY_COINBASE)
Test fix for BIP34 coinbase scriptSig length rule
No functional code change to validation or consensus logic
Evidence from the diff
The change clarifies BIP34 consensus rules: a coinbase scriptSig must be 2-100 bytes. It documents that TxIn::EMPTY_COINBASE intentionally violates this for PSBT compatibility, and fixes a test that was marked #[ignore] due to using an invalid empty coinbase scriptSig. The test now constructs a coinbase TxIn with a 2-byte script_sig (0x51 0x51).
Changed components
primitives/src/transaction.rsprimitives/src/block.rsInspect captured patch +18 / −7
diff --git a/primitives/src/block.rs b/primitives/src/block.rs
index 38203577..80def2a9 100644
--- a/primitives/src/block.rs
+++ b/primitives/src/block.rs
@@ -1162,7 +1162,6 @@ mod tests {
}
#[test]
- #[ignore] // bad test; will be fixed in next commit
#[cfg(feature = "alloc")]
fn block_decode() {
// Make a simple block, encode then decode. Verify equivalence.
@@ -1188,12 +1187,20 @@ mod tests {
};
let block: u32 = 741_521;
- let transactions = vec![Transaction {
- version: crate::transaction::Version::ONE,
- lock_time: units::absolute::LockTime::from_height(block).unwrap(),
- inputs: vec![crate::transaction::TxIn::EMPTY_COINBASE],
- outputs: Vec::new(),
- }];
+ let transactions = vec![
+ Transaction {
+ version: crate::transaction::Version::ONE,
+ lock_time: units::absolute::LockTime::from_height(block).unwrap(),
+ inputs: vec![crate::transaction::TxIn {
+ previous_output: crate::transaction::OutPoint::COINBASE_PREVOUT,
+ // Coinbase scriptSig must be 2-100 bytes
+ script_sig: crate::script::ScriptSigBuf::from_bytes(vec![0x51, 0x51]),
+ sequence: crate::sequence::Sequence::MAX,
+ witness: crate::witness::Witness::new(),
+ }],
+ outputs: Vec::new(),
+ },
+ ];
let original_block = Block::new_unchecked(header, transactions);
// Encode + decode the block
diff --git a/primitives/src/transaction.rs b/primitives/src/transaction.rs
index e69baa09..2346f31e 100644
--- a/primitives/src/transaction.rs
+++ b/primitives/src/transaction.rs
@@ -780,6 +780,10 @@ pub struct TxIn {
#[cfg(feature = "alloc")]
impl TxIn {
/// An empty transaction input with the previous output as for a coinbase transaction.
+ ///
+ /// This has a 0-byte scriptSig which is **invalid** per consensus rules
+ /// (coinbase scriptSig must be 2-100 bytes). This is kept for backwards compatibility
+ /// in PSBT workflows where the scriptSig is filled in later.
pub const EMPTY_COINBASE: Self = Self {
previous_output: OutPoint::COINBASE_PREVOUT,
script_sig: ScriptSigBuf::new(),
Why this scored 12/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.