feat: Wrap dlc messages with 10101 messages #2473
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
holzeis
force-pushed
the
feat/wrap-rust-dlc-messages-with-10101-messages
branch
from
096dd96
to
b2bc8f2
Compare
luckysori
changed the title
luckysori
approved these changes
Apr 26, 2024
There was a problem hiding this comment.
Large but straightforward diff. LGTM!
I know you wanted to talk about something, so maybe I've missed something in my review.
The only question I have is if this PR is backwards-compatible. This definitely looks like a breaking change to me, but I was wondering if we have to migrate anything at the database level, since we have a dlc_messages table. I guess not, since the PR passes CI.
I think we could label the 2nd commit as a refactor. Also, since we've been using conventional commits for a while, we could go a bit further with the commit message e.g.
refactor!: Wrap DLC messages with TenTenOneMessages
This is primarily a refactoring, moving the DLC message handler from the `rust-dlc` dependency to 10101, allowing us in following steps to enrich and adapt protocol messages with 10101 meta data.
BREAKING CHANGE: The format of the messages we send over the wire has changed.
| } | ||
|
|
||
| impl TenTenOneMessage { | ||
| pub fn get_reference_id(&self) -> Option<ReferenceId> { |
There was a problem hiding this comment.
🤡 Eventually we can actually move the reference ID onto the TenTenOneMessage.
This is primarily a refactoring, moving the DLC message handler from the `rust-dlc` dependency to 10101, allowing us in following steps to enrich and adapt protocol messages with 10101 metadata. BREAKING CHANGE: The format of the messages we send over the wire has changed.
holzeis
force-pushed
the
feat/wrap-rust-dlc-messages-with-10101-messages
branch
from
b2bc8f2
to
1ff6dcb
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is primarily a refactoring, moving the dlc message handler from the
rust-dlcdependency to 10101, allowing us in following steps to enrich and adapt protocol messages with 10101 meta data.I figured I already open a PR for this change, before adapting the messages with 10101 meta data. My aim is to create custom messages for 10101 custom flows
fixes #2380
fixes #2013