Skip to content

Codec for BOLT-07 gossip routing#25

Open
kushagra0902 wants to merge 4 commits intomorehouse:masterfrom
kushagra0902:master
Open

Codec for BOLT-07 gossip routing#25
kushagra0902 wants to merge 4 commits intomorehouse:masterfrom
kushagra0902:master

Conversation

@kushagra0902
Copy link
Copy Markdown

Key objectives achieved:

  • BOLT types applied: channel_announcement (256), node_announcement (257), channel_update (258).
  • Added/used gossip primitives: ChainHash, NodeId, ShortChannelId, Signature; wired message routing and wire encode/decode support.

Files changed/added:

  • smite/src/bolt/channel_announcement, channel_update, node_announcement with new codec
  • smite/src/bolt/wire.rs, smite/src/bolt/types.rs, smite/src/bolt.rs for wiring up new codecs

@kushagra0902
Copy link
Copy Markdown
Author

@morehouse In this PR, I tried to implement the codecs for Gossip Protocols and node discovery(Bolt-07). I am now planning to move further with reading and understanding more about gossip protocol and work on the "Gossip message fuzzing" project.
I will be grateful if you can review this PR and guide me on the next steps

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.

Please split this into three PRs: one per codec.

Please also make sure CI passes before requesting another review. You should be able to run all the CI commands locally (e.g., cargo test, cargo fmt, cargo clippy --all-targets --all-features -- -D warnings)

#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)]
pub struct ChainHash(pub [u8; CHAIN_HASH_SIZE]);

impl ChainHash {
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.

I think we can just continue using [u8; CHAIN_HASH_SIZE] for simplicity -- no need for this newtype.

#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub struct Signature(pub [u8; SIGNATURE_SIZE]);

impl Signature {
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 already are using secp256k1::ecdsa::Signature elsewhere, so I think we don't need to implement our own signature.

#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub struct NodeId(pub [u8; NODE_ID_SIZE]);

impl NodeId {
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 already use secp256k1::PublicKey for this elsewhere, so let's not implement our own.

#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)]
pub struct ShortChannelId(pub u64);

impl ShortChannelId {
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 just use u64 for simplicity?

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.

2 participants