Closed dariusc93 closed 1 month ago
lgtm.
You mentioned the lack backwards compatibility - are there any important migration steps? Is this only making sure none of the relay servers are outdated? Or are there other implications? I assume users won't lose any data and won't need to do anything.
Just in term of the data being transmitted so previous commits would not be compatible with this commit hence the change in topic names so any data transmitted would not repeatedly log an error due to using an older format. There are no migration needed since this is just related to the data being transmitted over the network and not impact local data itself.
What this PR does 📖
PayloadRequest
toPayloadMessage
PayloadBuilder
Payload
, replacing it withPayloadMessage
PayloadMessage::addresses
Which issue(s) this PR fixes 🔨
Special notes for reviewers 🗒️
Additional comments 🎤
Payload
was used originally to cut down on allocation when receiving bytes, however it acts more of a carrier and had less functionality and limitations while at that point it would of made more sense to either use protobuf for zero-copy or just allocate. Replacing it withPayloadMessage
(originallyPayloadRequest
), which was originally designed for delivering identity payload, allows us to sign and verify the message itself, cosign (related to #253), and be more functional with different options such as attaching peer address when a message is sent (which would be helpful for #253 and #389).PayloadMessage::addresses
is added as a means of providing the peer address(es) to allow one to connect in the event of content discovery. Related to #253, this will allow connecting to peers for content when the node public key differs from the identity public key. Additionally, messages are broadcast over a common topic while not directly connected to a peer, this would allow us to establish connection where possible or as needed.