Skip to content

bolt: implement tx_add_input message codec#23

Open
harsh04044 wants to merge 1 commit intomorehouse:masterfrom
harsh04044:bolt/tx-add-input
Open

bolt: implement tx_add_input message codec#23
harsh04044 wants to merge 1 commit intomorehouse:masterfrom
harsh04044:bolt/tx-add-input

Conversation

@harsh04044
Copy link
Copy Markdown

Implements the tx_add_input message codec (type 0x0042) for BOLT 2 interactive transaction construction.

The prevtx field carries a full consensus-encoded Bitcoin transaction, decoded using bitcoin::consensus::deserialize and length-prefixed on the wire via the WireFormat trait.

Includes encode/decode, Message enum integration, and tests covering roundtrip, truncation, and trailing bytes.

Part of #5

@harsh04044
Copy link
Copy Markdown
Author

@morehouse picked up tx_add_input since the embedded prevtx field made it the most involved of the interactive-tx codecs. Would love a review when you get a chance.

Copy link
Copy Markdown
Owner

@morehouse morehouse left a comment

Choose a reason for hiding this comment

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

Thanks for tackling this. Main feedback:

  • let's avoid depending on the bitcoin crate for now
  • let's add support for the tlvs field
  • please structure the tests similarly to the other codecs (open_channel is a good example)

Comment on lines +19 to +20
/// Previous transaction being spent (consensus-encoded on the wire).
pub prevtx: Transaction,
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

The dependency on bitcoin pulls a lot of new crates in.

At this point in the project I'd like to avoid that dependency -- let's make prevtx a Vec<u8> instead. Later on, when we are working on the interactive-tx flow, we can revisit whether it makes sense to take this dependency or implement our own minimal Transaction functionality.

pub prevtx_vout: u32,
/// The sequence number for this input.
pub sequence: u32,
}
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

We also should support the tlvs field.

}

#[test]
fn truncated_decode_returns_error() {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Ideally we should test truncation at every field (one test per field) like we do with other codecs.

@harsh04044
Copy link
Copy Markdown
Author

Updated: dropped the bitcoin crate dependency (prevtx is now Vec), added TxAddInputTlvs following the open_channel pattern, and restructured tests to match the other codecs with one test per truncated field.

@morehouse

@harsh04044 harsh04044 requested a review from morehouse April 7, 2026 10:32
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