Pull originator ID from AAD in XIP-49 #85
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
mchenani
approved these changes
Jan 31, 2025
fbac
approved these changes
Feb 2, 2025
mkysel
added a commit
to xmtp/xmtpd
that referenced
this pull request
Feb 10, 2025
Stable hashing in the payer to pick the correct node to send the write to. Hash is calculated based on the largest known ID and the topic. This also pulls the changes to protos to stop using the originator_id and use is_commit instead. Right now the client does not implement is_commit correctly. And hard codes the nodeID to 100 in all cases. In the case of off-chain messages, this still guarantees that the message was written on that particular node. In case of on-chain commits, the guarantee introduced in #418 somewhat holds. Since the client is no longer selecting a node, the payer will automatically detect which node should be seen as the preferred node and will block until that node has read the chain. In neither case can we guarantee that any other node has read any write. The stable hashing algorithm is pretty naive and might eventually need some enhancements. There are two issues that I see with it as is (which is the standard implementation): - if there are node ID gaps, the next nodeID gets double or more of the traffic - if we issue a new node ID it might rehash The payer does not yet have any retry logic, it does not check the liveness of the nodes, etc. All of that has to happen in a future PR. Relates to: - xmtp/XIPs#85 - xmtp/proto#242 Fixes xmtp/libxmtp#1455 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Summary by CodeRabbit - **New Features** - Introduced an enhanced routing mechanism that leverages stable hashing for more optimal node selection and balanced message distribution. - **Refactor** - Standardized how routing data is assigned, ensuring consistent handling of message originators. - Refined criteria for routing validation and blockchain publishing, improving overall reliability. - **Tests** - Expanded test coverage to verify stable node selection, robust message routing, and balanced topic assignments. - Enhanced test setup to utilize a mock registry for improved clarity and control over interactions. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
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 PR removes the responsibility of the client to determine a node to send the message to.
Instead, it becomes the payer's decision how to route messages in the system.
Remove the load balancing code and recommend stable hashing (ring hashing) instead.