refactor: reverse trait system#255
Merged
sedited merged 17 commits intofarcaster-project:mainfrom Jul 11, 2022
Merged
Conversation
first step to refactor serde in this library.
Simplify trait bounds on TaggedElement and take direct generic on AccordantKeys and AccordandKeySet instead of nested generic.
Move protocol_message into the new protocol module. Modify the generic argument to split the context into multiple generic. Add more derives on types. This commit prepare the removing of bundles from core. They are in fact messages created inside node already. It also prepare the migration of execution logic on Alice and Bob into the new protocol module.
Alice and Bob logic is migrated into protocol module.
This huge refactor rework all the trait system used in the library to push types to the surface instead of having one context: Swap. This allows more flexibility but refactor is not completly done, some trait should be removed and some should be split, e.g. crypto::Sign, to allow less generics in protocol functions. This refactor allows proper implementation/derivation on messages that are used in node and passed over the network.
sedited
reviewed
Jul 11, 2022
This was referenced Jul 11, 2022
sedited
approved these changes
Jul 11, 2022
Contributor
sedited
left a comment
sedited
left a comment
There was a problem hiding this comment.
I don't understand some things going on in this change, but I think I have a good overview of the set of changes now. I don't want to spend more time on this, so I will give my approval for this to go ahead. This has some clear benefits and I like that the "god-type" BtcXmr was removed and replaced with more fine grained controls.
I also feel like this allows us to eventually actually use the swap implementation as defined in swap/ without having to resort to many of the lower-level definitions.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Rework of the trait system used in the library to push everything at the surface. Allows better types handling in the future.
This introduces a list of surface breaking changes, to have a good idea see
tests/protocol.rswhich test the high level API of the lib.Remove global
CtxThe most significant change is removing the global context
Ctx: Swapwhich was defined on the typeBtcXmr, example of changes:or
These changes are applied to all
protocol::message(protocol_messageold) so it become possible and standard to e.g. usederive(..)macros.Remove bundles
The second significant change is the removal of
bundletypes. Bundles was design in RFCs as message exchanged between microservices. Those types are currently defined, as it should be, in the node directly.To not over-bloat this already bloated PR I'll now continue on separate PRs based on this one, so we can do a review process with @TheCharlatan.
Fix #252, fix #245
Resources: dtolay about "trait bounds on structs"