Skip to content

smite: add update_add_htlc message codec#33

Open
kartikangiras wants to merge 1 commit into
morehouse:masterfrom
kartikangiras:update-add-htlc
Open

smite: add update_add_htlc message codec#33
kartikangiras wants to merge 1 commit into
morehouse:masterfrom
kartikangiras:update-add-htlc

Conversation

@kartikangiras
Copy link
Copy Markdown

Add support for Message Type: [UPDATE_ADD_HTLC] (type 128).

realtes to #5

Comment thread smite/src/bolt/update_add_htlc.rs Outdated
Comment thread smite/src/bolt/update_add_htlc.rs
Comment thread smite/src/bolt/update_add_htlc.rs Outdated
Comment thread smite/src/bolt/update_add_htlc.rs
Comment thread smite/src/bolt/update_add_htlc.rs
Comment thread smite/src/bolt/update_add_htlc.rs
@morehouse
Copy link
Copy Markdown
Owner

CI doesn't pass

@kartikangiras
Copy link
Copy Markdown
Author

@morehouse I have locally tested each command on a linux based machine (it took time because i was on a macbook and it natively does not support these commands).

Comment thread smite/src/bolt/update_add_htlc.rs Outdated
Comment thread smite/src/bolt/update_add_htlc.rs Outdated
Comment thread smite/src/bolt/update_add_htlc.rs Outdated
Comment thread smite/src/bolt/update_add_htlc.rs Outdated
Comment thread smite/src/bolt/update_add_htlc.rs Outdated
Comment thread smite/src/bolt/update_add_htlc.rs Outdated
Comment thread smite/src/bolt.rs
Comment thread smite/src/bolt/update_add_htlc.rs
Comment thread smite/src/bolt/update_add_htlc.rs Outdated
@kartikangiras
Copy link
Copy Markdown
Author

@morehouse I have made all the necessary require changes as reviewed by you.

const ONION_PACKET_SIZE: usize = 1366;

/// TLV type for blinded path key.
const TLV_BLINDED_PATH: u64 = 3;
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.

This is the wrong TLV number.

#[derive(Debug, Clone, Default, PartialEq, Eq)]
pub struct UpdateAddHtlcTlvs {
/// Optionally specifies the blinded path key for this HTLC
pub blinded_path: Option<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.

This is a point, so we should be using PublicKey as the type.

let data = &encoded[..encoded.len() - 1];
// Should fail because TLV value is truncated
let decoded = UpdateAddHtlc::decode(data);
assert!(decoded.is_err());
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 verify the actual error returned, just like all the other error tests.

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