Add Missing Examples#412
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds one compilable example per module across access, math/core, and fixed-point packages, with matching tests and updated README example links. ChangesAccess examples
Math core examples
Fixed-point examples
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #412 +/- ##
==========================================
+ Coverage 96.26% 96.29% +0.03%
==========================================
Files 29 29
Lines 2968 2969 +1
Branches 732 732
==========================================
+ Hits 2857 2859 +2
Misses 66 66
+ Partials 45 44 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (1)
math/core/examples/decimal_scaling/tests/example_token_normalizer_tests.move (1)
32-162: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd the missing overflow regression.
This suite covers conversion, truncation, insufficient balance, and wrong-cap handling, but it never exercises the downcast-overflow path. A simple case is two
u64::MAXstable deposits: the aggregatednormalized_balancestill fits inu256, whilestable_balance()should overflow on the way back tou64. Without that check, the example misses one of the behaviors this layer is supposed to pin down.Based on PR objectives, this layer is expected to cover "conversion, truncation, overflow, and wrong-cap handling".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@math/core/examples/decimal_scaling/tests/example_token_normalizer_tests.move` around lines 32 - 162, The test suite is missing coverage for the downcast overflow path, so add a new regression in the existing `example_token_normalizer_tests.move` suite alongside the other `MultiAssetLedger` tests. Create a case that deposits two very large stable amounts (for example, near `u64::MAX`) so `normalized_balance()` still succeeds on the `u256` basis, then assert that calling `stable_balance()` triggers the expected overflow behavior when converting back to `u64`. Use the existing `ledger::to_normalized`, `deposit_stable`, and `stable_balance` flow to keep the test aligned with the current conversion coverage.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@contracts/access/examples/access_control/tests/example_reward_treasury_tests.move`:
- Around line 156-185: Add coverage for the root-admin renounce path in the
`timelocked_root_admin_handoff` test setup by exercising
`begin_default_admin_renounce` and the corresponding completion/expiry flow,
using the same `setup`, `AccessControl<EXAMPLE_REWARD_TREASURY>`, and `clock`
helpers already in this test. If you do not plan to test renounce here, update
the related documentation/comments so the claims about governance path coverage
no longer mention renounce.
In `@contracts/access/examples/delayed_transfer/example_timelocked_treasury.move`:
- Around line 33-40: The Treasury capability model is too broad because
TreasuryKey is not bound to a specific Treasury and withdraw/New do not verify
any association. Update the TreasuryKey and Treasury logic so each key records
or is otherwise tied to one Treasury, then make withdraw validate that the
supplied key matches the target Treasury before allowing funds to move; use the
TreasuryKey, Treasury::new, and Treasury::withdraw symbols to locate the
changes. Also add a regression test proving a key minted for one treasury cannot
withdraw from a different treasury.
In `@contracts/access/examples/two_step_transfer/example_operator_handoff.move`:
- Around line 38-41: The authorization in this example is too broad because
set_paused accepts any &OperatorCap, allowing a cap from one Service to affect
another. Update Service to store the expected OperatorCap identity when the
service is created, then in set_paused compare the provided cap against that
stored value before changing paused. Use the Service struct, the initialization
logic that creates the instance, and set_paused to wire the per-instance
capability binding.
In `@math/core/examples/decimal_scaling/example_token_normalizer.move`:
- Around line 63-71: The MultiAssetLedger design is incorrectly treating
heterogeneous assets as interchangeable by aggregating both deposits into one
normalized_balance, which only aligns decimals and not value. Update the example
around MultiAssetLedger and safe_upcast_balance to either model two decimal
representations of the same asset or split balances per asset and only aggregate
after an explicit pricing or unit-of-account conversion. Also adjust the README
guidance and any tests asserting the combined 6.5 result so they match the
revised model and use the relevant symbols MultiAssetLedger,
safe_upcast_balance, and the example_token_normalizer flow.
- Around line 95-100: The new() helper currently creates and shares
MultiAssetLedger in one step, which prevents callers from doing any setup before
sharing. Update new() to only construct and return the ledger or its admin cap,
and move transfer::share_object(ledger) out to a separate integration/entry
function so the example follows the create-then-share pattern. Keep the fix
localized around new() and the MultiAssetLedger / LedgerAdminCap flow.
In `@math/core/examples/integer_math/example_amm_quote.move`:
- Around line 80-88: The fee calculation in quote_swap_out can underflow when
fee_bps is greater than the basis-point maximum, so add a bounds check before
computing net_in. In quote_swap_out, validate fee_bps against the BPS limit and
abort with a dedicated error when it is out of range, then keep the net_in and
u64::mul_div flow unchanged for valid inputs.
In `@math/core/examples/vector/example_median_oracle.move`:
- Around line 62-73: The PriceOracle::report entry point is currently public to
everyone and should be gated by capability-based authorization instead of
allowing unrestricted mutation. Update the report method in
example_median_oracle.move to require a reporter/admin capability object
parameter (or equivalent capability held by the caller), and use that capability
to authorize appending prices and evicting old samples. Also adjust the example
tests and any call sites to construct and pass the capability so the authorized
flow is exercised.
- Around line 54-60: The new function currently constructs and shares the
PriceOracle internally, which prevents callers from controlling publication in
PTBs. Update new to only create and return the PriceOracle object (using
object::new and vector[] as today), and move the transfer::share_object call out
of this constructor so sharing is done by the caller. Keep the change localized
around new and PriceOracle so the API becomes composable without embedding
transfer behavior in object creation.
In `@math/fixed_point/examples/ud30x9/example_compounding.move`:
- Around line 77-80: The public API abort docs for the compounding example are
missing the overflow path from the `to_u64_trunc` conversion. Update the inline
doc-comments on the relevant public functions, including `balance_after` and
`interest_after`, to add `ud30x9_convert::EIntegerOverflow` alongside the
existing `ud30x9_base` aborts, since the fixed-point result can exceed
`u64::MAX` before truncation.
- Around line 28-32: The comment in the compounding example overstates exactness
for all inputs; `ud30x9::div`, `pow`, and `to_u64_trunc` can truncate for rates
or intermediates that are not exactly representable. Update the explanation near
the compounding walkthrough to limit the “exact” claim to the demonstrated small
integer rates/horizons (or explicitly say other `rate_num / rate_den` inputs are
deterministic but truncated), using `example_compounding` and the `ud30x9`
operations as the reference points.
In `@math/fixed_point/examples/ud30x9/tests/example_compounding_tests.move`:
- Line 97: Remove the bare abort from the compounding test since it is invalid
Move syntax and unnecessary here. Update the failing test logic in
example_compounding_tests.move so the expected_failure annotation on the
relevant test case is the only mechanism used to assert the abort path, keeping
the test body valid within the surrounding test function.
---
Nitpick comments:
In
`@math/core/examples/decimal_scaling/tests/example_token_normalizer_tests.move`:
- Around line 32-162: The test suite is missing coverage for the downcast
overflow path, so add a new regression in the existing
`example_token_normalizer_tests.move` suite alongside the other
`MultiAssetLedger` tests. Create a case that deposits two very large stable
amounts (for example, near `u64::MAX`) so `normalized_balance()` still succeeds
on the `u256` basis, then assert that calling `stable_balance()` triggers the
expected overflow behavior when converting back to `u64`. Use the existing
`ledger::to_normalized`, `deposit_stable`, and `stable_balance` flow to keep the
test aligned with the current conversion coverage.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a2a11588-2715-4927-a8ee-d241aced6099
📒 Files selected for processing (23)
contracts/README.mdcontracts/access/README.mdcontracts/access/examples/access_control/example_reward_treasury.movecontracts/access/examples/access_control/tests/example_reward_treasury_tests.movecontracts/access/examples/delayed_transfer/example_timelocked_treasury.movecontracts/access/examples/delayed_transfer/tests/example_timelocked_treasury_tests.movecontracts/access/examples/two_step_transfer/example_operator_handoff.movecontracts/access/examples/two_step_transfer/tests/example_operator_handoff_tests.movemath/README.mdmath/core/README.mdmath/core/examples/decimal_scaling/example_token_normalizer.movemath/core/examples/decimal_scaling/tests/example_token_normalizer_tests.movemath/core/examples/integer_math/example_amm_quote.movemath/core/examples/integer_math/tests/example_amm_quote_tests.movemath/core/examples/rounding/example_fee_split.movemath/core/examples/rounding/tests/example_fee_split_tests.movemath/core/examples/vector/example_median_oracle.movemath/core/examples/vector/tests/example_median_oracle_tests.movemath/fixed_point/README.mdmath/fixed_point/examples/sd29x9/example_zscore.movemath/fixed_point/examples/sd29x9/tests/example_zscore_tests.movemath/fixed_point/examples/ud30x9/example_compounding.movemath/fixed_point/examples/ud30x9/tests/example_compounding_tests.move
0xNeshi
reviewed
Jun 25, 2026
…lin/contracts-sui into feat/add-missing-examples-#386
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 (1)
contracts/access/examples/delayed_transfer/timelocked_treasury.move (1)
93-111: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winComplete the public API doc sections.
withdraw_wrappedcan abort throughwithdraw, but its doc block omits#### Aborts.availablealso omits the required#### Parametersand#### Returnssections. As per path instructions,STYLEGUIDE.mdsays public APIs and doc comments must include complete#### Parameters,#### Returns, and#### Abortssections when applicable.Suggested doc fix
/// Withdraw using a key that is still inside its delayed-transfer wrapper, borrowing it /// with the library's `borrow`. Operation does not wait on the custody timelock. /// /// #### Parameters /// - `self`: The treasury to draw from. /// - `wrapper`: The wrapper holding the treasury key. /// - `amount`: Units to withdraw. /// - `ctx`: Transaction context. /// /// #### Returns /// - A `Coin<SUI>` for `amount`. +/// +/// #### Aborts +/// - `EWrongTreasury` if the wrapped key is not bound to this treasury. public fun withdraw_wrapped(/// The treasury's current balance. +/// +/// #### Parameters +/// - `self`: The treasury to inspect. +/// +/// #### Returns +/// - The treasury's current `SUI` balance. public fun available(self: &Treasury): u64 {Also applies to: 115-118
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@contracts/access/examples/delayed_transfer/timelocked_treasury.move` around lines 93 - 111, The public API docs are incomplete: `withdraw_wrapped` needs an `#### Aborts` section because it can abort via `withdraw`, and `available` needs the required `#### Parameters` and `#### Returns` sections. Update the doc comments on `withdraw_wrapped` and `available` to match the STYLEGUIDE format for public APIs, using the existing function names to locate the blocks and adding the missing sections where applicable.Source: Path instructions
🧹 Nitpick comments (1)
contracts/access/examples/two_step_transfer/operator_handoff.move (1)
29-34: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd the missing
// === Imports ===section header.The file now follows STYLEGUIDE section blocks starting at
Errors, but theuseat Line 27 still sits outside the requiredImportssection, so the prescribed ordering is incomplete.As per coding guidelines, STYLEGUIDE.md requires section ordering using
// === <Name> ===blocks withImportsfirst. As per path instructions, review Move code against the conventions in STYLEGUIDE.md and the design rationale in ARCHITECTURE.md at the repository root - treat them as the single source of truth.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@contracts/access/examples/two_step_transfer/operator_handoff.move` around lines 29 - 34, The module’s section ordering is incomplete because the existing use statement is outside a required Imports block. Add a `// === Imports ===` header above the `use` declaration in `operator_handoff.move`, keeping the STYLEGUIDE section format consistent with the existing `// === Errors ===` block and ensuring Imports appears before later sections.Sources: Coding guidelines, Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@math/core/examples/decimal_scaling/token_normalizer.move`:
- Around line 136-139: The payout accounting in token_normalizer.move is
inconsistent with the documented dust invariant: `payout` currently subtracts
the full `normalized_amount` before truncating, so the dust is removed from
`normalized_balance` instead of staying in the ledger. Update the `payout` flow
(and any related example/docs/tests) so the semantics match the intended
behavior, either by deducting only the re-normalized native amount or by
revising the comments and assertions around `normalized_amount` and `payout` to
explicitly document dust forfeiture.
- Around line 99-207: The public API docs in token_normalizer.move are
incomplete, so add the missing `#### Parameters`, `#### Returns`, and `####
Aborts` sections where they apply. Update the doc comments for `new`,
`deposit_canonical`, `deposit_wrapped`, `payout_canonical`, `payout_wrapped`,
`normalized_balance`, `canonical_balance`, `wrapped_balance`, and
`to_normalized` to clearly document each argument, the returned value (or lack
of one), and all abort conditions, matching the existing style used by the
module and the helper symbols like `deposit`, `payout`, and
`decimal_scaling::safe_downcast_balance`.
- Around line 39-40: The import block in token_normalizer.move is missing the
required STYLEGUIDE section header. Add the `// === Imports ===` section marker
immediately before the existing `use` statements, and keep the imports grouped
under that block to match the required Move section ordering convention.
---
Outside diff comments:
In `@contracts/access/examples/delayed_transfer/timelocked_treasury.move`:
- Around line 93-111: The public API docs are incomplete: `withdraw_wrapped`
needs an `#### Aborts` section because it can abort via `withdraw`, and
`available` needs the required `#### Parameters` and `#### Returns` sections.
Update the doc comments on `withdraw_wrapped` and `available` to match the
STYLEGUIDE format for public APIs, using the existing function names to locate
the blocks and adding the missing sections where applicable.
---
Nitpick comments:
In `@contracts/access/examples/two_step_transfer/operator_handoff.move`:
- Around line 29-34: The module’s section ordering is incomplete because the
existing use statement is outside a required Imports block. Add a `// ===
Imports ===` header above the `use` declaration in `operator_handoff.move`,
keeping the STYLEGUIDE section format consistent with the existing `// ===
Errors ===` block and ensuring Imports appears before later sections.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6e44012a-03aa-4b6c-9107-1bb3cec05e98
📒 Files selected for processing (21)
contracts/access/README.mdcontracts/access/examples/access_control/reward_treasury.movecontracts/access/examples/access_control/tests/reward_treasury_tests.movecontracts/access/examples/delayed_transfer/tests/timelocked_treasury_tests.movecontracts/access/examples/delayed_transfer/timelocked_treasury.movecontracts/access/examples/two_step_transfer/operator_handoff.movecontracts/access/examples/two_step_transfer/tests/operator_handoff_tests.movemath/core/README.mdmath/core/examples/decimal_scaling/tests/token_normalizer_tests.movemath/core/examples/decimal_scaling/token_normalizer.movemath/core/examples/integer_math/amm_quote.movemath/core/examples/integer_math/tests/amm_quote_tests.movemath/core/examples/rounding/fee_split.movemath/core/examples/rounding/tests/fee_split_tests.movemath/core/examples/vector/median_oracle.movemath/core/examples/vector/tests/median_oracle_tests.movemath/fixed_point/README.mdmath/fixed_point/examples/sd29x9/tests/zscore_tests.movemath/fixed_point/examples/sd29x9/zscore.movemath/fixed_point/examples/ud30x9/compounding.movemath/fixed_point/examples/ud30x9/tests/compounding_tests.move
💤 Files with no reviewable changes (6)
- math/fixed_point/examples/sd29x9/tests/zscore_tests.move
- math/core/examples/rounding/fee_split.move
- math/fixed_point/examples/ud30x9/tests/compounding_tests.move
- math/core/examples/rounding/tests/fee_split_tests.move
- contracts/access/examples/access_control/reward_treasury.move
- math/fixed_point/examples/sd29x9/zscore.move
✅ Files skipped from review due to trivial changes (5)
- math/core/examples/integer_math/tests/amm_quote_tests.move
- contracts/access/examples/two_step_transfer/tests/operator_handoff_tests.move
- math/fixed_point/README.md
- contracts/access/README.md
- math/core/examples/vector/median_oracle.move
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
4 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.
Resolves #386
Summary by CodeRabbit