Deploy to production - #817
Merged
Merged
Conversation
…ERC7984, and Stellar Fungible/Stablecoin (#812)
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro 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)
Comment |
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Eric Lau <ericglau@outlook.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/core/stellar/src/fungible.ts (1)
255-267:⚠️ Potential issue | 🟠 Major | ⚡ Quick winClamp premint to
i128, notu128.This path validates the scaled premint as
u128, but bothBase::mintandFungibleVotes::mintreceiveamount: i128in this file. A premint that fitsu128but exceedsi128::MAXwill still be emitted into the constructor and then fail when the generated Rust code is compiled. The new configurabledecimalsmakes that overflow easier to hit after scaling.Proposed fix
function addPremint(c: ContractBuilder, amount: string, decimals: bigint, votes: boolean) { if (amount !== undefined && amount !== '0') { if (!premintPattern.test(amount)) { throw new OptionsError({ premint: 'Not a valid number', }); } // TODO: handle signed int? - const premintAbsolute = toUint(getInitialSupply(amount, Number(decimals)), 'premint', 'u128'); + const premintAbsolute = toUint(getInitialSupply(amount, Number(decimals)), 'premint', 'u128'); + if (premintAbsolute > (1n << 127n) - 1n) { + throw new OptionsError({ + premint: 'Value is greater than i128 max value', + }); + } c.addConstructorArgument({ name: 'recipient', type: 'Address' }); c.addConstructorCode(`${votes ? 'FungibleVotes' : 'Base'}::mint(e, &recipient, ${premintAbsolute});`); } }🤖 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 `@packages/core/stellar/src/fungible.ts` around lines 255 - 267, The premint scaling currently uses toUint(...) and emits a u128 literal, but Base::mint/FungibleVotes::mint expect i128, so change the validation/conversion in addPremint to use a signed 128-bit conversion (e.g. toInt or equivalent) on getInitialSupply(amount, Number(decimals)) and produce an i128 value (replace the toUint call and the u128 target with the signed equivalent) so the generated constructor argument/premintAbsolute matches i128 range and will fail early if it exceeds i128::MAX.packages/ui/src/confidential/ERC7984Controls.svelte (1)
27-38:⚠️ Potential issue | 🟡 MinorEnforce ERC7984 wrappable invariants continuously (not just on
wrappabletoggle)In
packages/ui/src/confidential/ERC7984Controls.sveltethe only reset ofopts.decimals/opts.premintruns whenopts.wrappable !== previousWrappable; ifdecimalsorpremintchange whileopts.wrappablestaystrue, those invalid values can persist behind disabled inputs.buildERC7984rejectswrappablewith non-defaultdecimalsand incompatiblepremint, so the wizard can end up in an error state with hidden invalid options.Suggested fix
let savedDecimals = opts.decimals; let savedPremint = opts.premint; let previousWrappable = opts.wrappable; $: if (opts.wrappable !== previousWrappable) { if (opts.wrappable) { savedDecimals = opts.decimals; savedPremint = opts.premint; - opts.decimals = erc7984.defaults.decimals; - opts.premint = ''; } else { opts.decimals = savedDecimals; opts.premint = savedPremint; } previousWrappable = opts.wrappable; } + +$: if (opts.wrappable) { + opts.decimals = erc7984.defaults.decimals; + opts.premint = ''; +}🤖 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 `@packages/ui/src/confidential/ERC7984Controls.svelte` around lines 27 - 38, The reactive block currently only runs when opts.wrappable changes, so invalid opts.decimals or opts.premint can persist while opts.wrappable stays true; modify the logic around opts.wrappable/previousWrappable/savedDecimals/savedPremint so that whenever opts.wrappable is true you actively enforce the invariants (e.g., if opts.wrappable then ensure opts.decimals is set to erc7984.defaults.decimals and opts.premint is cleared or validated) and when opts.wrappable becomes false restore savedDecimals/savedPremint; in short, change the reactive check to run whenever opts.wrappable OR the dependent fields (opts.decimals, opts.premint) change so the invariant is continuously enforced and buildERC7984 will not receive invalid hidden values.
🧹 Nitpick comments (5)
packages/common/src/ai/descriptions/stellar.ts (1)
34-37: ⚡ Quick win
stellarStablecoinDescriptions.decimalsis added but not wired into the schema contract.
stellarStablecoinSchemacurrently inheritsdecimalsfromstellarFungibleSchema, so this new stablecoin-specific description won’t be surfaced. Either remove this duplicate description key or explicitly overridedecimalsinstellarStablecoinSchemato usestellarStablecoinDescriptions.decimals.Proposed schema-side fix
export const stellarStablecoinSchema = { ...stellarFungibleSchema, + decimals: z.string().optional().describe(stellarStablecoinDescriptions.decimals), limitations: z .literal(false) .or(z.literal('allowlist')) .or(z.literal('blocklist'))🤖 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 `@packages/common/src/ai/descriptions/stellar.ts` around lines 34 - 37, stellarStablecoinDescriptions adds a decimals description but stellarStablecoinSchema currently inherits decimals from stellarFungibleSchema so the new description is never used; update stellarStablecoinSchema to explicitly set/override the decimals field to use stellarStablecoinDescriptions.decimals (or remove the duplicate key from stellarStablecoinDescriptions if you prefer) so the stablecoin schema surfaces the intended description, referencing stellarStablecoinSchema, stellarFungibleSchema and stellarStablecoinDescriptions.decimals when making the change.packages/core/solidity/src/utils/convert-strings.test.ts (1)
45-52: ⚡ Quick winAdd a
uint64boundary case here.This suite exercises
uint8anduint256, but the new helper also backs ERC7984'suint64limits in this cohort. A baduint64max entry would currently slip through this shared test file.♻️ Suggested test addition
+test('toUint - uint64 max', t => { + t.is(toUint('18446744073709551615', 'foo', 'uint64'), BigInt('18446744073709551615')); +}); + +test('toUint - uint64 too large', t => { + const error = t.throws(() => toUint('18446744073709551616', 'foo', 'uint64'), { + instanceOf: OptionsError, + }); + t.is(error.messages.foo, 'Value is greater than uint64 max value'); +}); + test('toUint - uint8', t => { t.is(toUint('255', 'foo', 'uint8'), BigInt(255)); });🤖 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 `@packages/core/solidity/src/utils/convert-strings.test.ts` around lines 45 - 52, Add tests covering the uint64 boundary in the same file to ensure toUint enforces ERC7984 uint64 limits: add a passing case using toUint('18446744073709551615', 'foo', 'uint64') expecting BigInt(18446744073709551615) and a failing case calling toUint('18446744073709551616', 'foo', 'uint64') asserting it throws an OptionsError with error.messages.foo indicating the value is greater than uint64 max value; reference the existing test names and patterns (e.g., the 'toUint - uint8' and 'toUint - uint8 too large' tests) and the toUint and OptionsError symbols to place and implement these new assertions.packages/core/confidential/src/generate/erc7984.ts (1)
1-10: ⚡ Quick winDerive blueprint decimals from the exported builder constants.
'6'and'10'duplicate the runtime contract inerc7984.ts, so a later change toDEFAULT_DECIMALSorMAX_DECIMALScan desync generated fixtures from builder validation.♻️ Proposed fix
-import { DEFAULT_DECIMALS, type ERC7984Options } from '../erc7984'; +import { DEFAULT_DECIMALS, MAX_DECIMALS, type ERC7984Options } from '../erc7984'; @@ - decimals: ['6', '10'], + decimals: [DEFAULT_DECIMALS.toString(), MAX_DECIMALS.toString()],🤖 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 `@packages/core/confidential/src/generate/erc7984.ts` around lines 1 - 10, Replace the hardcoded decimals entries in the blueprint object with values derived from the exported builder constants: use DEFAULT_DECIMALS and MAX_DECIMALS (convert to strings) instead of the literal '6' and '10'. Update the decimals field in the blueprint (variable name: blueprint, property: decimals) to compute its array from the imported constants so future changes to DEFAULT_DECIMALS/MAX_DECIMALS stay in sync with the generated fixtures and the ERC7984 builder validation.packages/core/confidential/src/erc7984.test.ts (1)
139-151: ⚡ Quick winAdd one premint-overflow case with non-default decimals.
The new overflow path now depends on the configured
decimals, but the current assertions only lock the default-6 case. Adecimals: '10'overflow test would cover the branch this change introduced.Also applies to: 179-192
🤖 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 `@packages/core/confidential/src/erc7984.test.ts` around lines 139 - 151, Add a second test case alongside the existing "erc7984 premint more precise than decimals" that specifically exercises the overflow branch for non-default decimals: call buildERC7984 with decimals: '10' and a premint string that has more precision or magnitude than allowed by 10 decimals (e.g., a value that would overflow when scaled), then assert (error as OptionsError).messages.premint equals 'Too many decimals' (mirror the existing assertion). Do the same addition for the similar test block referenced around the other case (the block at 179-192) so both the precision and overflow branches for custom decimals are covered, using the same identifiers buildERC7984, OptionsError and messages.premint to locate the tests.packages/core/confidential/src/zip-hardhat.test.ts (1)
45-65: ⚡ Quick winExercise a non-default decimals fixture in the Hardhat integration test.
This suite is the compile-time safety net for emitted contracts, but it still only covers the default-decimals path. Adding one
decimals !== '6'case would validate the newdecimals()override and premint scaling end to end.🤖 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 `@packages/core/confidential/src/zip-hardhat.test.ts` around lines 45 - 65, The test currently only exercises the default decimals path; add another serial test (similar to the existing test.serial block) that builds an ERC7984 options set with decimals set to a non-default value (e.g., decimals: '8') and appropriate premint value to validate premint scaling, then call buildERC7984(opts) and await runIgnitionTest(c, t); reference the same types (ERC7984Options, GenericOptions) and functions (buildERC7984, runIgnitionTest, test.serial) so the Hardhat integration covers the decimals() override end-to-end.
🤖 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.
Outside diff comments:
In `@packages/core/stellar/src/fungible.ts`:
- Around line 255-267: The premint scaling currently uses toUint(...) and emits
a u128 literal, but Base::mint/FungibleVotes::mint expect i128, so change the
validation/conversion in addPremint to use a signed 128-bit conversion (e.g.
toInt or equivalent) on getInitialSupply(amount, Number(decimals)) and produce
an i128 value (replace the toUint call and the u128 target with the signed
equivalent) so the generated constructor argument/premintAbsolute matches i128
range and will fail early if it exceeds i128::MAX.
In `@packages/ui/src/confidential/ERC7984Controls.svelte`:
- Around line 27-38: The reactive block currently only runs when opts.wrappable
changes, so invalid opts.decimals or opts.premint can persist while
opts.wrappable stays true; modify the logic around
opts.wrappable/previousWrappable/savedDecimals/savedPremint so that whenever
opts.wrappable is true you actively enforce the invariants (e.g., if
opts.wrappable then ensure opts.decimals is set to erc7984.defaults.decimals and
opts.premint is cleared or validated) and when opts.wrappable becomes false
restore savedDecimals/savedPremint; in short, change the reactive check to run
whenever opts.wrappable OR the dependent fields (opts.decimals, opts.premint)
change so the invariant is continuously enforced and buildERC7984 will not
receive invalid hidden values.
---
Nitpick comments:
In `@packages/common/src/ai/descriptions/stellar.ts`:
- Around line 34-37: stellarStablecoinDescriptions adds a decimals description
but stellarStablecoinSchema currently inherits decimals from
stellarFungibleSchema so the new description is never used; update
stellarStablecoinSchema to explicitly set/override the decimals field to use
stellarStablecoinDescriptions.decimals (or remove the duplicate key from
stellarStablecoinDescriptions if you prefer) so the stablecoin schema surfaces
the intended description, referencing stellarStablecoinSchema,
stellarFungibleSchema and stellarStablecoinDescriptions.decimals when making the
change.
In `@packages/core/confidential/src/erc7984.test.ts`:
- Around line 139-151: Add a second test case alongside the existing "erc7984
premint more precise than decimals" that specifically exercises the overflow
branch for non-default decimals: call buildERC7984 with decimals: '10' and a
premint string that has more precision or magnitude than allowed by 10 decimals
(e.g., a value that would overflow when scaled), then assert (error as
OptionsError).messages.premint equals 'Too many decimals' (mirror the existing
assertion). Do the same addition for the similar test block referenced around
the other case (the block at 179-192) so both the precision and overflow
branches for custom decimals are covered, using the same identifiers
buildERC7984, OptionsError and messages.premint to locate the tests.
In `@packages/core/confidential/src/generate/erc7984.ts`:
- Around line 1-10: Replace the hardcoded decimals entries in the blueprint
object with values derived from the exported builder constants: use
DEFAULT_DECIMALS and MAX_DECIMALS (convert to strings) instead of the literal
'6' and '10'. Update the decimals field in the blueprint (variable name:
blueprint, property: decimals) to compute its array from the imported constants
so future changes to DEFAULT_DECIMALS/MAX_DECIMALS stay in sync with the
generated fixtures and the ERC7984 builder validation.
In `@packages/core/confidential/src/zip-hardhat.test.ts`:
- Around line 45-65: The test currently only exercises the default decimals
path; add another serial test (similar to the existing test.serial block) that
builds an ERC7984 options set with decimals set to a non-default value (e.g.,
decimals: '8') and appropriate premint value to validate premint scaling, then
call buildERC7984(opts) and await runIgnitionTest(c, t); reference the same
types (ERC7984Options, GenericOptions) and functions (buildERC7984,
runIgnitionTest, test.serial) so the Hardhat integration covers the decimals()
override end-to-end.
In `@packages/core/solidity/src/utils/convert-strings.test.ts`:
- Around line 45-52: Add tests covering the uint64 boundary in the same file to
ensure toUint enforces ERC7984 uint64 limits: add a passing case using
toUint('18446744073709551615', 'foo', 'uint64') expecting
BigInt(18446744073709551615) and a failing case calling
toUint('18446744073709551616', 'foo', 'uint64') asserting it throws an
OptionsError with error.messages.foo indicating the value is greater than uint64
max value; reference the existing test names and patterns (e.g., the 'toUint -
uint8' and 'toUint - uint8 too large' tests) and the toUint and OptionsError
symbols to place and implement these new assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: fe8f4306-b148-43e5-be8e-2b5fffe3aac4
⛔ Files ignored due to path filters (7)
packages/cli/src/cli.test.ts.snapis excluded by!**/*.snappackages/core/confidential/src/erc7984.test.ts.snapis excluded by!**/*.snappackages/core/solidity/src/erc20.test.ts.snapis excluded by!**/*.snappackages/core/solidity/src/stablecoin.test.ts.snapis excluded by!**/*.snappackages/core/stellar/src/fungible.test.ts.snapis excluded by!**/*.snappackages/core/stellar/src/stablecoin.test.ts.snapis excluded by!**/*.snappackages/mcp/src/confidential/tools/erc7984.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (66)
.changeset/bump-stellar-contracts.md.changeset/erc7984-wrappable-premint.md.changeset/hardhat-3-sample-project.mdpackages/cli/CHANGELOG.mdpackages/cli/package.jsonpackages/cli/src/cli.test.ts.mdpackages/common/CHANGELOG.mdpackages/common/package.jsonpackages/common/src/ai/descriptions/confidential.tspackages/common/src/ai/descriptions/solidity.tspackages/common/src/ai/descriptions/stellar.tspackages/common/src/ai/schemas/confidential.tspackages/common/src/ai/schemas/solidity.tspackages/common/src/ai/schemas/stellar.tspackages/core/confidential/CHANGELOG.mdpackages/core/confidential/package.jsonpackages/core/confidential/src/erc7984.test.tspackages/core/confidential/src/erc7984.test.ts.mdpackages/core/confidential/src/erc7984.tspackages/core/confidential/src/generate/erc7984.tspackages/core/confidential/src/zip-hardhat.test.tspackages/core/solidity/CHANGELOG.mdpackages/core/solidity/package.jsonpackages/core/solidity/src/erc20.test.tspackages/core/solidity/src/erc20.test.ts.mdpackages/core/solidity/src/erc20.tspackages/core/solidity/src/generate/erc20.tspackages/core/solidity/src/generate/stablecoin.tspackages/core/solidity/src/index.tspackages/core/solidity/src/stablecoin.test.tspackages/core/solidity/src/stablecoin.test.ts.mdpackages/core/solidity/src/utils/convert-strings.test.tspackages/core/solidity/src/utils/convert-strings.tspackages/core/stellar/CHANGELOG.mdpackages/core/stellar/package.jsonpackages/core/stellar/src/fungible.test.tspackages/core/stellar/src/fungible.test.ts.mdpackages/core/stellar/src/fungible.tspackages/core/stellar/src/generate/fungible.tspackages/core/stellar/src/generate/stablecoin.tspackages/core/stellar/src/stablecoin.test.tspackages/core/stellar/src/stablecoin.test.ts.mdpackages/mcp/CHANGELOG.mdpackages/mcp/package.jsonpackages/mcp/src/confidential/tools/erc7984.test.tspackages/mcp/src/confidential/tools/erc7984.test.ts.mdpackages/mcp/src/confidential/tools/erc7984.tspackages/mcp/src/solidity/tools/erc20.test.tspackages/mcp/src/solidity/tools/erc20.tspackages/mcp/src/solidity/tools/rwa.test.tspackages/mcp/src/solidity/tools/rwa.tspackages/mcp/src/solidity/tools/stablecoin.test.tspackages/mcp/src/solidity/tools/stablecoin.tspackages/mcp/src/stellar/tools/fungible.test.tspackages/mcp/src/stellar/tools/fungible.tspackages/mcp/src/stellar/tools/stablecoin.test.tspackages/mcp/src/stellar/tools/stablecoin.tspackages/ui/api/ai-assistant/function-definitions/confidential.tspackages/ui/api/ai-assistant/function-definitions/solidity.tspackages/ui/api/ai-assistant/function-definitions/stellar.tspackages/ui/src/confidential/ERC7984Controls.sveltepackages/ui/src/solidity/ERC20Controls.sveltepackages/ui/src/solidity/RealWorldAssetControls.sveltepackages/ui/src/solidity/StablecoinControls.sveltepackages/ui/src/stellar/FungibleControls.sveltepackages/ui/src/stellar/StablecoinControls.svelte
💤 Files with no reviewable changes (3)
- .changeset/hardhat-3-sample-project.md
- .changeset/erc7984-wrappable-premint.md
- .changeset/bump-stellar-contracts.md
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Includes #812