feat(standards): builder APIs for Swap, Burn, and Mint notes#10
Closed
VAIBHAVJINDAL3012 wants to merge 3 commits into
Closed
feat(standards): builder APIs for Swap, Burn, and Mint notes#10VAIBHAVJINDAL3012 wants to merge 3 commits into
VAIBHAVJINDAL3012 wants to merge 3 commits into
Conversation
* feat(testing): introduce TestTransactionBuilder * Simplify TestTransactionBuilder doc comment * fix: lint check
* feat(standards): add P2idNote bon builder, require at least one asset Replace the `P2idNote` marker type and its `P2idNote::create` factory with a `P2idNote` struct built via a `bon` typestate builder (`P2idNote::builder()`): - `.sender()`, `.target()`, and a serial number (`.serial_number()` or `.generate_serial_number(rng)`) are required; `.note_type()` defaults to private and attachments are optional. - `.asset()`/`.assets()` and `.attachment()`/`.attachments()` append items. - P2ID notes must now carry at least one asset, enforced in `new()` via the new `NoteError::MissingAsset`. - `From<P2idNote> for Note` converts infallibly. Migrate all `P2idNote::create` call sites to the builder. Incidental 0-asset P2ID notes in kernel tests move to P2ANY (which legitimately carries no assets), preserving the 0-asset coverage. Part of 0xMiden#2283 * docs(standards): address P2ID builder review comments - Drop the builder usage description from `P2idNote::new` docs; the typestate builder is self-documenting. - Trim redundant tails from the builder extension method docs. - Remove the "Use P2ANY ..." rationalization comments across the kernel tests and the P2ANY reference in the `setup_test` doc comment. - Assert the note type against `NoteType::default()` instead of a hardcoded `NoteType::Private` in the minimal-builder test. - Add a `compile_fail` doctest showing `.serial_number()` and `.generate_serial_number()` are mutually exclusive at compile time. * docs(standards): drop the dual-serial compile_fail doctest Per review feedback, remove the `compile_fail` doctest (and its explanation) on `generate_serial_number`; the typestate already enforces that the serial number cannot be set twice. * fix(standards): address 0xMiden#3126 review feedback - Use `NoteError::other` instead of adding a dedicated `MissingAsset` variant to the protocol crate; the "at least one asset" rule is a standards concern. - Remove the hand-rolled `create_p2id_note_exact` helper and inline the builder (with `.serial_number()`) at all 9 call sites. - Drop the low-value `builder_generates_serial_number` and `into_note_preserves_assets` tests; exercise `generate_serial_number` inside `builder_accumulates_assets` instead. - Remove the 5 duplicate CHANGELOG entries the rebase re-inserted. * fix(testing): remove unused `Word` import Left over from removing `create_p2id_note_exact`; clippy (-D warnings) flagged it.
Replace the `SwapNote`, `BurnNote`, and `MintNote` marker types and their `::create` factories with structs built via `bon` typestate builders, mirroring the merged `P2idNote` builder. - `BurnNote` and `MintNote` convert into a `Note` via `Note::from`. - `SwapNote` returns the outgoing note plus its payback `NoteDetails` via `SwapNote::into_notes`. - Per-note validations are preserved via `NoteError::other` (Swap: requested asset must differ from offered; Mint: faucet ID must match the embedded asset's faucet). - Migrate the mock-chain `add_swap_note` and the faucet burn/mint test sites. Part of 0xMiden#2283.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Author
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.
Superseded by the per-note PRs #11 (Swap), #12 (Burn), #13 (Mint).