-
Notifications
You must be signed in to change notification settings - Fork 10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[WIP] Broadcast {channel,node} information for multihop payment #144
Conversation
Fix compilation because of message change Convert between molecule and rust types Add AnnoucementSignature to message union use a dedicated type for announced node name Broadcast node announcement messages Refactor peer id in channel state Save peer pubkeys to channel Make signature field in channel {announcement, update} optional Get channel announcement message from channel state Return complete tx status on TraceTx Do not panic on node announcement command Define CFNBroadcastMessage for broadcast messages
#[derive(Debug)] | ||
pub struct TraceTxResponse { | ||
pub tx: Option<ckb_jsonrpc_types::TransactionView>, | ||
pub status: ckb_jsonrpc_types::TxStatus, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's better to import ckb_jsonrpc_types
, since there are multiple references.
src/fiber/channel.rs
Outdated
&self, | ||
state: &mut ChannelActorState, | ||
message: FiberMessage, | ||
message: FiberChannelNormalOperationMessage, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FiberChannelMessage
is better?
) | ||
.await; | ||
// Wait for the channel announcement to be broadcasted | ||
tokio::time::sleep(tokio::time::Duration::from_millis(5000)).await; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems need to add assertion here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. I was trying to add an expected event so that we can be sure this is working. As I hinted on the discord server, I think it would be better if we can just see all the messages sent to the network actor. I had some problem implementing some kind of introspection functionality. I can add a NetworkServiceEvent
for ChannelAnnouncement
here, but I found NetworkServiceEvent
had became somewhat bloated.
No description provided.