Skip to content

A couple improvements to BIP 54 test vectors#110

Open
darosior wants to merge 4 commits intobitcoin-inquisition:29.xfrom
darosior:2603_inquisition_bip54_tests_improvements
Open

A couple improvements to BIP 54 test vectors#110
darosior wants to merge 4 commits intobitcoin-inquisition:29.xfrom
darosior:2603_inquisition_bip54_tests_improvements

Conversation

@darosior
Copy link

Following up on some review feedback from #99 and from bitcoin/bips#2015, this:

  • ensures newline before EOF in generated test vectors;
  • corrects a typo pointed out by Chris Stewart;
  • adds a test case requested by Antoine Riard.

Copy link

@hodlinator hodlinator left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 27957d2

I support including a transaction spending Taproot-inputs that (after stripping the witness data) ends up at the invalid 64 bytes.

Copy link

@ariard ariard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Light Code Review ACK fc24db4

RecordTestCase(test_vectors, CTransaction{tx_copy}, /*valid=*/false, "A 64-byte Segwit transaction (1 p2tr input, 1 p2a output).");
}

// A semi-realistic 64-byte transaction: 1 Taproot input with an annex, 1 OP_RETURN output.
Copy link

@ariard ariard Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe a test case for a segwit v2+ to verify the validity is not changed by the novel BIP54 checks.

ofc, no reason why it would change anything for v2 - v16.

don’t remember if there is already another test case for it elsewhere.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants