feat: Enhance MorphTransaction for reference key#19
Open
chengwenxi wants to merge 6 commits intomainfrom
Open
Conversation
📝 WalkthroughWalkthroughThis change introduces versioning and extended metadata support to Morph transactions and receipts by adding version (u8), reference (Option), and memo (Option) fields with version-aware (V0 and V1) encoding/decoding logic, validation rules, and updated RLP extraction in the runtime integration layer. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
chengwenxi
requested review from
anylots and
panos-xyz
and removed request for
anylots
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
crates/primitives/src/receipt/mod.rs (1)
522-553:⚠️ Potential issue | 🟡 MinorGuard against out-of-range compact
versionvalues.Casting
u64→u8can silently truncate if stored data is corrupted or future versions exceed 255. A small guard avoids wraparound.🛠️ Suggested guard
- version: version.map(|v| v as u8), + version: version.and_then(|v| { + if v <= u8::MAX as u64 { Some(v as u8) } else { None } + }),crates/primitives/src/transaction/morph_transaction.rs (1)
768-817:⚠️ Potential issue | 🟡 MinorAvoid silent truncation when decoding compact
version.Casting
u64→u8can wrap on corrupted or unexpected data. Consider a safe fallback.🛠️ Suggested guard
- version: version as u8, + version: u8::try_from(version).unwrap_or(MORPH_TX_VERSION_0),
🤖 Fix all issues with AI agents
In `@crates/primitives/src/receipt/receipt.rs`:
- Around line 177-185: The is_morph_tx predicate currently checks fee_token_id,
version, and reference but misses memo, so receipts with only memo set are not
treated as Morph; update the pub const fn is_morph_tx(&self) -> bool in
receipt.rs to also include self.memo.is_some() in the boolean expression
(alongside self.fee_token_id.is_some(), self.version.is_some(), and
self.reference.is_some()) so memo presence is considered when detecting a
TxMorph.
In `@crates/primitives/src/transaction/morph_transaction.rs`:
- Around line 202-205: The is_v1 method currently uses a ">=" comparison which
treats newer versions as V1; change its logic in the is_v1 function to check
equality against MORPH_TX_VERSION_1 (use "==" with MORPH_TX_VERSION_1) so only
the exact supported version is considered V1, ensuring encoding/decoding
behavior matches.
- Around line 172-193: The validate_version method currently allows Version 0
MorphTx with reference or memo set even though V0 encoding drops those fields;
update MorphTx::validate_version to also reject requests where self.version ==
MORPH_TX_VERSION_0 and (self.reference is non-empty OR self.memo is non-empty)
by returning an Err with a clear message (e.g., "version 0 MorphTx must not
include reference or memo") so in-memory state cannot silently lose data when
roundtripping; keep existing FeeTokenID check and existing handling for
MORPH_TX_VERSION_1 and other versions.
Comment on lines
177
to
185
| /// Returns true if this receipt is for a TxMorph. | ||
| pub const fn is_morph_tx(&self) -> bool { | ||
| self.fee_token_id.is_some() | ||
| self.fee_token_id.is_some() || self.version.is_some() || self.reference.is_some() | ||
| } | ||
|
|
||
| /// Returns true if this receipt has a reference. | ||
| pub const fn has_reference(&self) -> bool { | ||
| self.reference.is_some() | ||
| } |
There was a problem hiding this comment.
Include memo in is_morph_tx detection.
A receipt with only memo set would be treated as non-Morph. Including it keeps the predicate robust to future construction paths.
🛠️ Suggested update
- self.fee_token_id.is_some() || self.version.is_some() || self.reference.is_some()
+ self.fee_token_id.is_some()
+ || self.version.is_some()
+ || self.reference.is_some()
+ || self.memo.is_some()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| /// Returns true if this receipt is for a TxMorph. | |
| pub const fn is_morph_tx(&self) -> bool { | |
| self.fee_token_id.is_some() | |
| self.fee_token_id.is_some() || self.version.is_some() || self.reference.is_some() | |
| } | |
| /// Returns true if this receipt has a reference. | |
| pub const fn has_reference(&self) -> bool { | |
| self.reference.is_some() | |
| } | |
| /// Returns true if this receipt is for a TxMorph. | |
| pub const fn is_morph_tx(&self) -> bool { | |
| self.fee_token_id.is_some() | |
| || self.version.is_some() | |
| || self.reference.is_some() | |
| || self.memo.is_some() | |
| } | |
| /// Returns true if this receipt has a reference. | |
| pub const fn has_reference(&self) -> bool { | |
| self.reference.is_some() | |
| } |
🤖 Prompt for AI Agents
In `@crates/primitives/src/receipt/receipt.rs` around lines 177 - 185, The
is_morph_tx predicate currently checks fee_token_id, version, and reference but
misses memo, so receipts with only memo set are not treated as Morph; update the
pub const fn is_morph_tx(&self) -> bool in receipt.rs to also include
self.memo.is_some() in the boolean expression (alongside
self.fee_token_id.is_some(), self.version.is_some(), and
self.reference.is_some()) so memo presence is considered when detecting a
TxMorph.
Comment on lines
+172
to
+193
| /// Validates the MorphTx version and its associated field requirements. | ||
| /// | ||
| /// Rules: | ||
| /// - Version 0 (legacy format): FeeTokenID must be > 0 | ||
| /// - Version 1 (with Reference/Memo): FeeTokenID, FeeLimit, Reference, Memo are all optional | ||
| /// - Other versions: not supported | ||
| pub fn validate_version(&self) -> Result<(), &'static str> { | ||
| match self.version { | ||
| MORPH_TX_VERSION_0 => { | ||
| // Version 0 requires FeeTokenID > 0 (legacy format used for alt-fee transactions) | ||
| if self.fee_token_id == 0 { | ||
| return Err("version 0 MorphTx requires FeeTokenID > 0"); | ||
| } | ||
| } | ||
| MORPH_TX_VERSION_1 => { | ||
| // Version 1: FeeTokenID, FeeLimit, Reference, Memo are all optional | ||
| // No additional validation needed | ||
| } | ||
| _ => { | ||
| return Err("unsupported MorphTx version"); | ||
| } | ||
| } |
There was a problem hiding this comment.
Reject reference/memo on V0 to avoid silent data loss.
V0 encoding ignores reference/memo; allowing them produces an in-memory state that won’t roundtrip over the wire.
🛠️ Suggested validation
MORPH_TX_VERSION_0 => {
// Version 0 requires FeeTokenID > 0 (legacy format used for alt-fee transactions)
if self.fee_token_id == 0 {
return Err("version 0 MorphTx requires FeeTokenID > 0");
}
+ if self.reference.is_some() || self.memo.is_some() {
+ return Err("version 0 MorphTx must not include reference or memo");
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| /// Validates the MorphTx version and its associated field requirements. | |
| /// | |
| /// Rules: | |
| /// - Version 0 (legacy format): FeeTokenID must be > 0 | |
| /// - Version 1 (with Reference/Memo): FeeTokenID, FeeLimit, Reference, Memo are all optional | |
| /// - Other versions: not supported | |
| pub fn validate_version(&self) -> Result<(), &'static str> { | |
| match self.version { | |
| MORPH_TX_VERSION_0 => { | |
| // Version 0 requires FeeTokenID > 0 (legacy format used for alt-fee transactions) | |
| if self.fee_token_id == 0 { | |
| return Err("version 0 MorphTx requires FeeTokenID > 0"); | |
| } | |
| } | |
| MORPH_TX_VERSION_1 => { | |
| // Version 1: FeeTokenID, FeeLimit, Reference, Memo are all optional | |
| // No additional validation needed | |
| } | |
| _ => { | |
| return Err("unsupported MorphTx version"); | |
| } | |
| } | |
| /// Validates the MorphTx version and its associated field requirements. | |
| /// | |
| /// Rules: | |
| /// - Version 0 (legacy format): FeeTokenID must be > 0 | |
| /// - Version 1 (with Reference/Memo): FeeTokenID, FeeLimit, Reference, Memo are all optional | |
| /// - Other versions: not supported | |
| pub fn validate_version(&self) -> Result<(), &'static str> { | |
| match self.version { | |
| MORPH_TX_VERSION_0 => { | |
| // Version 0 requires FeeTokenID > 0 (legacy format used for alt-fee transactions) | |
| if self.fee_token_id == 0 { | |
| return Err("version 0 MorphTx requires FeeTokenID > 0"); | |
| } | |
| if self.reference.is_some() || self.memo.is_some() { | |
| return Err("version 0 MorphTx must not include reference or memo"); | |
| } | |
| } | |
| MORPH_TX_VERSION_1 => { | |
| // Version 1: FeeTokenID, FeeLimit, Reference, Memo are all optional | |
| // No additional validation needed | |
| } | |
| _ => { | |
| return Err("unsupported MorphTx version"); | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@crates/primitives/src/transaction/morph_transaction.rs` around lines 172 -
193, The validate_version method currently allows Version 0 MorphTx with
reference or memo set even though V0 encoding drops those fields; update
MorphTx::validate_version to also reject requests where self.version ==
MORPH_TX_VERSION_0 and (self.reference is non-empty OR self.memo is non-empty)
by returning an Err with a clear message (e.g., "version 0 MorphTx must not
include reference or memo") so in-memory state cannot silently lose data when
roundtripping; keep existing FeeTokenID check and existing handling for
MORPH_TX_VERSION_1 and other versions.
Comment on lines
+202
to
+205
| /// Returns true if this is a version 1 MorphTx (with Reference/Memo). | ||
| pub const fn is_v1(&self) -> bool { | ||
| self.version >= MORPH_TX_VERSION_1 | ||
| } |
There was a problem hiding this comment.
is_v1 should match the supported version exactly.
>= will treat version 2+ as V1 for encoding, but decoding rejects them. Tighten to == until more versions are supported.
🛠️ Suggested fix
- pub const fn is_v1(&self) -> bool {
- self.version >= MORPH_TX_VERSION_1
- }
+ pub const fn is_v1(&self) -> bool {
+ self.version == MORPH_TX_VERSION_1
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| /// Returns true if this is a version 1 MorphTx (with Reference/Memo). | |
| pub const fn is_v1(&self) -> bool { | |
| self.version >= MORPH_TX_VERSION_1 | |
| } | |
| /// Returns true if this is a version 1 MorphTx (with Reference/Memo). | |
| pub const fn is_v1(&self) -> bool { | |
| self.version == MORPH_TX_VERSION_1 | |
| } |
🤖 Prompt for AI Agents
In `@crates/primitives/src/transaction/morph_transaction.rs` around lines 202 -
205, The is_v1 method currently uses a ">=" comparison which treats newer
versions as V1; change its logic in the is_v1 function to check equality against
MORPH_TX_VERSION_1 (use "==" with MORPH_TX_VERSION_1) so only the exact
supported version is considered V1, ensuring encoding/decoding behavior matches.
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
1 participant
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.
Summary by CodeRabbit
Release Notes
New Features
Documentation