Skip to content
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

feature: split Transaction from MessagePayload #483

Merged
merged 2 commits into from
Oct 18, 2023

Conversation

Ma233
Copy link
Member

@Ma233 Ma233 commented Oct 17, 2023

Please check if the PR fulfills these requirements

  • Tests for the changes have been added (for bug fixes/features)
  • Docs have been added / updated (for bug fixes / features)

🔵 What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

This PR moves the message packing and verification logic from MessagePayload to Transaction.
Now, the MessagePayload only works for transferring messages.
This PR also removed the generic type T from the MessagePayload struct since it does not care about the type when transferring and verifying. Instead, the T type was moved to the new method, and the data is serialized immediately upon creating the struct.

🟤 What is the current behavior? (You can also link to an open issue here)

  1. The origin_verification field and verification field appear strangely in MessagePayload.
  2. When users want to create passing messages, they have to create fake or empty MessageRelay.
  3. The generic type T for MessagePayload seems straightforward. And make it harder to be specified.
  4. The addr field is ambiguous, you have to understand it from the comments.
pub struct MessagePayload<T> {
    pub data: T,
    pub tx_id: uuid::Uuid,
    pub addr: Did,
    pub relay: MessageRelay,
    pub verification: MessageVerification,
    pub origin_verification: MessageVerification,
}

🟢 What is the new behavior (if this is a feature change)?

The Transaction that is used to hold a verified message can be constructed separately.
Users can create the MessagePayload at a later time when they want to transfer a Transaction.

pub struct Transaction {
    pub destination: Did,
    pub tx_id: uuid::Uuid,
    pub data: Vec<u8>,
    pub verification: MessageVerification,
}

pub struct MessagePayload {
    pub transaction: Transaction,
    pub relay: MessageRelay,
    pub verification: MessageVerification,
}

☢️ Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)

No breaking change.

ℹ️ Other information

Closes #issue

Copy link
Member

@RyanKung RyanKung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Ma233 Ma233 merged commit a6cbc42 into RingsNetwork:master Oct 18, 2023
3 checks passed
@Ma233 Ma233 deleted the payload branch October 18, 2023 10:16
r2d2-rs pushed a commit that referenced this pull request Nov 8, 2023
 feat: add support for curve secp256r1 (NistP256) (#486) 
 feat!: general client for FFI and WASM (#478) 
 refactor!: separated transport crate (#471) 
 refactor:add SwarmCallback to allow user make delayed callback binding (#485)
 refactor: split pool from transport and add BoxedTransport (#479) 
 refactor: split Transaction from MessagePayload (#483) 
 chore: remove web3 dependency and fix Did display prefix (#480)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants