Skip to content

bolt: implement tx_add_output message codec#26

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

bolt: implement tx_add_output message codec#26
harsh04044 wants to merge 1 commit intomorehouse:masterfrom
harsh04044:bolt/tx-add-output

Conversation

@harsh04044
Copy link
Copy Markdown

Implements the tx_add_output codec (type 67) for BOLT 2 interactive-tx.

The script field is a variable-length scriptPubKey, length-prefixed via the WireFormat trait. Tests cover roundtrip, empty script, trailing bytes, and truncation at each field boundary.

Part of #5.

Copy link
Copy Markdown

@kushagra0902 kushagra0902 left a comment

Choose a reason for hiding this comment

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

The PR looks good to me with everything handled properly

@Vineet1101
Copy link
Copy Markdown

The PR looks good to me with everything handled properly

bro how can you say that. I am trying to run it locally and my code is not even compiling
image
The implementation for TxAddOutput imports bitcoin::ScriptBuf, but the rust-bitcoin crate isn't actually a dependency in smite's workspace.

@harsh04044 harsh04044 force-pushed the bolt/tx-add-output branch 2 times, most recently from ecec0ed to eb6d3d4 Compare April 5, 2026 09:45
@harsh04044
Copy link
Copy Markdown
Author

harsh04044 commented Apr 5, 2026

The PR looks good to me with everything handled properly

bro how can you say that. I am trying to run it locally and my code is not even compiling image The implementation for TxAddOutput imports bitcoin::ScriptBuf, but the rust-bitcoin crate isn't actually a dependency in smite's workspace.

Good catch. The bitcoin crate was added as part of the tx_add_input PR (#23) which hasn't merged yet. Added it directly to this branch -- should compile now.
@Vineet1101

@kushagra0902
Copy link
Copy Markdown

The PR looks good to me with everything handled properly

bro how can you say that. I am trying to run it locally and my code is not even compiling image The implementation for TxAddOutput imports bitcoin::ScriptBuf, but the rust-bitcoin crate isn't actually a dependency in smite's workspace.

Good catch. The bitcoin crate was added as part of the tx_add_input PR (#23) which hasn't merged yet. Added it directly to this branch -- should compile now. @Vineet1101

I was working at something else and had the package already that bypassed the error, good catch @Vineet1101

/// The value of this output in satoshis.
pub sats: u64,
/// The scriptPubKey for this output.
pub script: ScriptBuf,
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.

Let's use script: Vec<u8> for now to avoid the bitcoin dependency. We can add it later if necessary.

impl TxAddOutput {
/// Encodes to wire format (without message type prefix).
#[must_use]
pub fn encode(&self) -> Vec<u8> {
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 should assert that script.len() <= u16::MAX here since Vec<u8>::write assumes this.

})
);
}

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 should also test the cases where the script length is truncated or the script itself is truncated.

@harsh04044 harsh04044 force-pushed the bolt/tx-add-output branch from eb6d3d4 to 643d73c Compare April 7, 2026 11:01
@harsh04044
Copy link
Copy Markdown
Author

Done, script is now Vec, added the assert on script.len() in encode(), and split out truncation tests for the script length and script data fields.
@morehouse

@harsh04044 harsh04044 requested a review from morehouse April 7, 2026 11:04
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.

4 participants