Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughComprehensive upgrade and refactoring across the Compact smart contracts framework: bumps language version to 0.21.0, restructures simulator architecture using base factory pattern, renames token transfer parameters from Changes
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
🚥 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 |
This reverts commit 3af8a05.
emnul
changed the title
emnul
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/compact/src/Builder.ts (1)
13-107:⚠️ Potential issue | 🟠 MajorBuilder still accepts flags despite BuilderOptions omitting them.
BuilderOptionsexcludesflags, but the constructor/fromArgs still acceptCompilerOptionsand pass flags through toCompactCompiler, allowing--skip-zkto slip in. If builds must always include ZK proofs, strip or reject flags and align the examples.🔧 Suggested fix (type + flag stripping)
- constructor(options: CompilerOptions = {}) { + constructor(options: BuilderOptions = {}) { this.options = options; } static fromArgs( args: string[], env: NodeJS.ProcessEnv = process.env, ): CompactBuilder { - const options = CompactCompiler.parseArgs(args, env); - return new CompactBuilder(options); + const { flags: _flags, ...builderOptions } = + CompactCompiler.parseArgs(args, env); + return new CompactBuilder(builderOptions); }packages/compact/src/runCompiler.ts (1)
133-140:⚠️ Potential issue | 🟡 MinorHandle
--src/--outparse errors the same way as--dir.
parseArgsnow throws for missing--src/--outvalues, but those fall into the unexpected-error path and skip usage help.🔧 Suggested fix
- if (errorMessage.includes('--dir flag requires a directory name')) { - spinner.fail( - chalk.red('[COMPILE] Error: --dir flag requires a directory name'), - ); + if ( + errorMessage.includes('--dir flag requires a directory name') || + errorMessage.includes('--src flag requires a directory path') || + errorMessage.includes('--out flag requires a directory path') + ) { + spinner.fail(chalk.red(`[COMPILE] Error: ${errorMessage}`)); showUsageHelp(); return; }
🤖 Fix all issues with AI agents
In `@contracts/src/token/FungibleToken.compact`:
- Line 519: Implement the missing shieldedBurnAddress helper inside the Utils
module and export it with the project's existing naming convention, then update
all call sites to use that exported name; specifically, add a function in
Utils.compact (e.g., export as Utils_shieldedBurnAddress or if your Utils module
exposes an object, Utils_.shieldedBurnAddress) that returns the burn address
(matching expected return type used by FungibleToken.compact,
NonFungibleToken.compact, MultiToken.compact, Ownable.compact) and replace the
bare shieldedBurnAddress() calls in those files with the new Utils-prefixed
identifier.
In `@contracts/src/token/test/MultiToken.test.ts`:
- Around line 511-529: The test sets approvals for two different operators
(Z_SPENDER and Z_OTHER) but both transfers call ._unsafeTransferFrom via
token.as(caller) so Z_OTHER is never used; update the transfers to perform the
first transfer using token.as(Z_SPENDER)._unsafeTransferFrom(...) and the second
transfer using token.as(Z_OTHER)._unsafeTransferFrom(...) (keeping the existing
_mint, setApprovalForAll, and balanceOf assertions) so the test exercises two
distinct approved operators.
- Around line 306-324: The test mistakenly uses the same operator for both
transfers so it never verifies a second distinct spender; update the second
transfer to use the other approved spender (replace the second
token.as(SPENDER).transferFrom(...) call with token.as(Z_OTHER or an OTHER
constant).transferFrom(...)) and ensure an OTHER constant is defined if needed;
keep the approvals via token.as(caller).setApprovalForAll(Z_SPENDER, true) and
token.as(caller).setApprovalForAll(Z_OTHER, true) and the initial mint via
token._mint(Z_OWNER, TOKEN_ID, AMOUNT * 2n) unchanged.
In `@contracts/src/token/test/simulators/NonFungibleTokenSimulator.ts`:
- Around line 65-71: Update the JSDoc for the symbol() method in
NonFungibleTokenSimulator: change the `@returns` description from "The token
name." to "The token symbol." so it matches the `@description`. Locate the public
symbol(): string { return this.circuits.impure.symbol(); } method and adjust its
docblock accordingly.
In `@contracts/src/token/test/utils/address.ts`:
- Around line 36-46: The function encodeToAddress currently passes a hex string
to isContractAddress but the API expects an Either<ZswapCoinPublicKey,
ContractAddress>; change the check to construct or obtain that Either (e.g. use
createEitherTestContractAddress(generatedAddress) or the appropriate Either
factory) and pass that Either into isContractAddress, then if valid call
encodeContractAddress on the ContractAddress contained in the Either (extracting
the right value) before returning bytes; keep the existing error throw for
invalid inputs and preserve use of toHexPadded and encodeContractAddress.
In `@packages/compact/README.md`:
- Line 26: Update the example version strings that currently read "Compactc
version: 0.29.0" to "Compactc version: 0.28.0" so the README matches the PR
target (upgrade-contracts-versions-0.28.0); search for occurrences of the
literal "Compactc version: 0.29.0" in the README (including the instances shown
in the diff and around lines indicated) and replace each with "Compactc version:
0.28.0".
In `@packages/compact/src/Builder.ts`:
- Around line 66-83: The build steps in Builder (the steps array) hardcode paths
like "src/artifacts" and "dist", causing mismatch with configured src/out
directories; update Builder to derive those copy/move commands from the instance
options (e.g., use options.srcDir and options.outDir or the compiler-resolved
directories) and replace the hardcoded paths in the steps entries for "Copying
artifacts" and "Copying and cleaning .compact files" so the shell commands
reference the resolved src/out locations; ensure the mkdir/cp/find commands use
the derived paths and preserve the existing shell flags and fallbacks.
In `@packages/simulator/test/fixtures/utils/address.ts`:
- Around line 32-43: encodeToAddress may produce oversized hex when
toHexPadded(str) returns >64 chars; ensure you clamp the payload explicitly
before prefixing: compute hex = toHexPadded(str), then truncate/clamp hex to at
most 60 hex characters (30 bytes) for the part that you will prepend
PREFIX_ADDRESS to, so that PREFIX_ADDRESS + truncated is always 64 hex chars;
keep using encodeContractAddress on the resulting fixed-length string and
reference encodeToAddress, toHexPadded, PREFIX_ADDRESS, and
encodeContractAddress when making the change.
In `@packages/simulator/test/setup.ts`:
- Around line 26-45: The current isSpawnSyncRet type guard requires
SpawnSyncReturns-specific fields (error, status) which prometified exec errors
don't have; update isSpawnSyncRet to return true for either the SpawnSyncReturns
shape or the exec-style error shape by checking stdout/stderr (string|Buffer)
plus one of: (typedErr.error instanceof Error) OR exec-style properties like
typeof typedErr.code === 'number' || typeof typedErr.code === 'string' ||
typedErr.code === null, typeof typedErr.killed === 'boolean', and (typeof
typedErr.signal === 'string' || typedErr.signal === null) and optionally typeof
typedErr.cmd === 'string'; use these symbol names (isSpawnSyncRet,
SpawnSyncReturns, promisify(exec)) to locate and replace the guard logic so exec
rejection objects are recognized and handled instead of re-thrown.
In `@packages/simulator/test/unit/core/StateManager.test.ts`:
- Around line 122-125: Replace the spread+cast pattern that loses WASM backing
by constructing a proper QueryContext instance: instead of creating
modifiedTxCtx with {...ctx.currentQueryContext, address:
encodeToAddress('otherAddress')} as QueryContext, call the QueryContext
constructor with the original state and the new address (use
ctx.currentQueryContext.state and encodeToAddress('otherAddress')) so the
resulting object retains the WASM __wbg_ptr backing.
🧹 Nitpick comments (9)
contracts/src/archive/test/ShieldedToken.test.ts (1)
55-63: Nitpick: Duplicate assertion forShieldedToken__counter.Lines 58 and 62 both check the same value (
ShieldedToken__counter === 0n). Consider removing the duplicate.🔧 Suggested fix
it('should set public state', () => { token = new ShieldedTokenSimulator(NONCE, NAME, SYMBOL, DECIMALS); expect(token.getCurrentPublicState().ShieldedToken__counter).toEqual(0n); expect(token.getCurrentPublicState().ShieldedToken__domain).toEqual( DOMAIN, ); - expect(token.getCurrentPublicState().ShieldedToken__counter).toEqual(0n); });contracts/src/token/test/MultiToken.test.ts (1)
219-228: Inconsistent use of.as(caller)pattern in setApprovalForAll tests.Lines 220, 223, and 226 call
token.setApprovalForAll(...)directly without the.as(caller)wrapper, while thebeforeEachat line 212 and other tests in this file use.as(caller). This inconsistency may cause the test to rely on implicit simulator state rather than explicitly setting the caller context.Suggested fix for consistency
it('should unset → set → unset operator', () => { - token.setApprovalForAll(Z_SPENDER, false); + token.as(caller).setApprovalForAll(Z_SPENDER, false); expect(token.isApprovedForAll(Z_OWNER, Z_SPENDER)).toBe(false); - token.setApprovalForAll(Z_SPENDER, true); + token.as(caller).setApprovalForAll(Z_SPENDER, true); expect(token.isApprovedForAll(Z_OWNER, Z_SPENDER)).toBe(true); - token.setApprovalForAll(Z_SPENDER, false); + token.as(caller).setApprovalForAll(Z_SPENDER, false); expect(token.isApprovedForAll(Z_OWNER, Z_SPENDER)).toBe(false); });packages/compact/README.md (1)
67-77: Consider adding language specification to code fences.The directory structure code blocks lack language specification, which some Markdown linters flag. Consider adding
textorplaintextmarkers for better tooling compatibility:📝 Suggested formatting improvement
For the first block:
-``` +```text src/ access/ AccessControl.compactApply the same pattern to the second directory tree block at lines 81-87.
Also applies to: 81-87
packages/simulator/test/unit/core/StateManager.test.ts (1)
67-78: Remove assertions on__wbg_ptrimplementation detail.
__wbg_ptris a WebAssembly binding internal pointer. Testing for its presence and type makes tests fragile across library updates without providing functional validation. The existing assertions on instance types and public properties (address,state) are sufficient.Proposed simplification
it('should set original state', () => { expect(ctx.currentQueryContext).toBeInstanceOf(QueryContext); - expect(ctx.currentQueryContext).toHaveProperty('__wbg_ptr'); - expect((ctx.currentQueryContext as any).__wbg_ptr).toBeTypeOf('number'); }); it('should set tx ctx', () => { // Need to go deeper expect(ctx.currentQueryContext).toBeInstanceOf(QueryContext); expect(ctx.currentQueryContext.address).toEqual(dummyContractAddress()); expect(ctx.currentQueryContext.state).toBeInstanceOf(ChargedState); - expect(ctx.currentQueryContext.state).toHaveProperty('__wbg_ptr'); });.github/ISSUE_TEMPLATE/01_bug_report.yml (1)
95-97: Consider adding previous versions to the dropdown.The dropdown only contains the new default version. Users encountering bugs on older versions won't be able to accurately report which version they're using. Consider keeping a few recent versions as options.
💡 Suggested improvement
options: - 0.29.0 (Default) + - 0.28.0 + - 0.26.0 default: 0packages/simulator/test/setup.ts (1)
71-86: Consider adding a timeout to the exec call.The
execAsync(command)call has no timeout, which could cause the test setup to hang indefinitely if thecompacttool stalls. This can block CI pipelines.⏱️ Proposed fix to add timeout
- const command = `compact compile --skip-zk "${inputPath}" "${outputDir}"`; + const command = `compact compile --skip-zk "${inputPath}" "${outputDir}"`; + const COMPILE_TIMEOUT_MS = 60_000; // 60 seconds try { - await execAsync(command); + await execAsync(command, { timeout: COMPILE_TIMEOUT_MS }); } catch (err: unknown) {contracts/src/archive/test/utils/address.ts (1)
37-41: Add a comment explaining why the archive version uses a different encoding approach.The archive implementation at lines 37-41 is indeed different from the four other
encodeToAddressimplementations in the codebase (token, utils, access, and security modules). Those all usetoHexPadded()withisContractAddressvalidation and throw on invalid input. The archive version uniquely usesBuffer.from(str, 'ascii').toString('hex')withPREFIX_ADDRESSand skips validation entirely. Consider adding an inline comment documenting whether this difference is intentional for backward compatibility with legacy contract data.contracts/src/security/test/utils/address.ts (1)
1-109: Consider consolidating duplicate address utility files.This file is nearly identical to
contracts/src/token/test/utils/address.ts. Consider extracting common utilities to a shared location to reduce maintenance burden. However, this may be intentional for test module isolation.contracts/src/token/test/simulators/NonFungibleTokenSimulator.ts (1)
450-452: Consider adding explicit type annotation forprivateState.The property lacks an explicit type annotation, so TypeScript infers it as
{}. For consistency withNonFungibleTokenPrivateState(which isRecord<string, never>), consider adding the type explicitly.📝 Proposed fix
- public readonly privateState = {}; + public readonly privateState: NonFungibleTokenPrivateState = {};
| it('should handle concurrent operations on same token ID', () => { | ||
| token._mint(Z_OWNER, TOKEN_ID, AMOUNT * 2n); | ||
|
|
||
| // Set up two spenders | ||
| token.setApprovalForAll(Z_SPENDER, true, caller); | ||
| token.setApprovalForAll(Z_OTHER, true, caller); | ||
| token.as(caller).setApprovalForAll(Z_SPENDER, true); | ||
| token.as(caller).setApprovalForAll(Z_OTHER, true); | ||
|
|
||
| // First spender transfers half | ||
| token.transferFrom(Z_OWNER, Z_RECIPIENT, TOKEN_ID, AMOUNT, SPENDER); | ||
| token | ||
| .as(SPENDER) | ||
| .transferFrom(Z_OWNER, Z_RECIPIENT, TOKEN_ID, AMOUNT); | ||
| expect(token.balanceOf(Z_RECIPIENT, TOKEN_ID)).toEqual(AMOUNT); | ||
|
|
||
| // Second spender transfers remaining | ||
| token.transferFrom(Z_OWNER, Z_RECIPIENT, TOKEN_ID, AMOUNT, SPENDER); | ||
| token | ||
| .as(SPENDER) | ||
| .transferFrom(Z_OWNER, Z_RECIPIENT, TOKEN_ID, AMOUNT); | ||
| expect(token.balanceOf(Z_RECIPIENT, TOKEN_ID)).toEqual(AMOUNT * 2n); | ||
| }); |
There was a problem hiding this comment.
Test does not use the second spender as intended.
The test sets up two spenders (Z_SPENDER and Z_OTHER) at lines 310-311, but both transfers at lines 314-316 and 320-322 use SPENDER. The comment at line 319 says "Second spender transfers remaining" but the code doesn't actually use the second operator. This means the test doesn't verify that multiple distinct operators can perform transfers.
Suggested fix
// Second spender transfers remaining
token
- .as(SPENDER)
+ .as(utils.toHexPadded('OTHER'))
.transferFrom(Z_OWNER, Z_RECIPIENT, TOKEN_ID, AMOUNT);Alternatively, define an OTHER constant similar to SPENDER:
const OTHER = utils.toHexPadded('OTHER');🤖 Prompt for AI Agents
In `@contracts/src/token/test/MultiToken.test.ts` around lines 306 - 324, The test
mistakenly uses the same operator for both transfers so it never verifies a
second distinct spender; update the second transfer to use the other approved
spender (replace the second token.as(SPENDER).transferFrom(...) call with
token.as(Z_OTHER or an OTHER constant).transferFrom(...)) and ensure an OTHER
constant is defined if needed; keep the approvals via
token.as(caller).setApprovalForAll(Z_SPENDER, true) and
token.as(caller).setApprovalForAll(Z_OTHER, true) and the initial mint via
token._mint(Z_OWNER, TOKEN_ID, AMOUNT * 2n) unchanged.
| it('should handle concurrent operations on same token ID', () => { | ||
| token._mint(Z_OWNER, TOKEN_ID, AMOUNT * 2n); | ||
|
|
||
| // Set up two spenders | ||
| token.setApprovalForAll(Z_SPENDER, true, caller); | ||
| token.setApprovalForAll(Z_OTHER, true, caller); | ||
| token.as(caller).setApprovalForAll(Z_SPENDER, true); | ||
| token.as(caller).setApprovalForAll(Z_OTHER, true); | ||
|
|
||
| // First spender transfers half | ||
| token._unsafeTransferFrom( | ||
| Z_OWNER, | ||
| recipient, | ||
| TOKEN_ID, | ||
| AMOUNT, | ||
| SPENDER, | ||
| ); | ||
| token | ||
| .as(caller) | ||
| ._unsafeTransferFrom(Z_OWNER, recipient, TOKEN_ID, AMOUNT); | ||
| expect(token.balanceOf(recipient, TOKEN_ID)).toEqual(AMOUNT); | ||
|
|
||
| // Second spender transfers remaining | ||
| token._unsafeTransferFrom( | ||
| Z_OWNER, | ||
| recipient, | ||
| TOKEN_ID, | ||
| AMOUNT, | ||
| SPENDER, | ||
| ); | ||
| token | ||
| .as(caller) | ||
| ._unsafeTransferFrom(Z_OWNER, recipient, TOKEN_ID, AMOUNT); | ||
| expect(token.balanceOf(recipient, TOKEN_ID)).toEqual(AMOUNT * 2n); | ||
| }); |
There was a problem hiding this comment.
Test does not use the second spender as intended.
Similar to the transferFrom test, this test sets up two spenders (Z_SPENDER and Z_OTHER) but both transfers use caller (either OWNER or SPENDER depending on the parameterized test). The second approved operator Z_OTHER is never actually used to perform a transfer.
Suggested fix
To properly test concurrent operations with multiple operators, the second transfer should use a different spender:
// Second spender transfers remaining
token
- .as(caller)
+ .as(utils.toHexPadded('OTHER'))
._unsafeTransferFrom(Z_OWNER, recipient, TOKEN_ID, AMOUNT);📝 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.
| it('should handle concurrent operations on same token ID', () => { | |
| token._mint(Z_OWNER, TOKEN_ID, AMOUNT * 2n); | |
| // Set up two spenders | |
| token.setApprovalForAll(Z_SPENDER, true, caller); | |
| token.setApprovalForAll(Z_OTHER, true, caller); | |
| token.as(caller).setApprovalForAll(Z_SPENDER, true); | |
| token.as(caller).setApprovalForAll(Z_OTHER, true); | |
| // First spender transfers half | |
| token._unsafeTransferFrom( | |
| Z_OWNER, | |
| recipient, | |
| TOKEN_ID, | |
| AMOUNT, | |
| SPENDER, | |
| ); | |
| token | |
| .as(caller) | |
| ._unsafeTransferFrom(Z_OWNER, recipient, TOKEN_ID, AMOUNT); | |
| expect(token.balanceOf(recipient, TOKEN_ID)).toEqual(AMOUNT); | |
| // Second spender transfers remaining | |
| token._unsafeTransferFrom( | |
| Z_OWNER, | |
| recipient, | |
| TOKEN_ID, | |
| AMOUNT, | |
| SPENDER, | |
| ); | |
| token | |
| .as(caller) | |
| ._unsafeTransferFrom(Z_OWNER, recipient, TOKEN_ID, AMOUNT); | |
| expect(token.balanceOf(recipient, TOKEN_ID)).toEqual(AMOUNT * 2n); | |
| }); | |
| it('should handle concurrent operations on same token ID', () => { | |
| token._mint(Z_OWNER, TOKEN_ID, AMOUNT * 2n); | |
| // Set up two spenders | |
| token.as(caller).setApprovalForAll(Z_SPENDER, true); | |
| token.as(caller).setApprovalForAll(Z_OTHER, true); | |
| // First spender transfers half | |
| token | |
| .as(caller) | |
| ._unsafeTransferFrom(Z_OWNER, recipient, TOKEN_ID, AMOUNT); | |
| expect(token.balanceOf(recipient, TOKEN_ID)).toEqual(AMOUNT); | |
| // Second spender transfers remaining | |
| token | |
| .as(Z_OTHER) | |
| ._unsafeTransferFrom(Z_OWNER, recipient, TOKEN_ID, AMOUNT); | |
| expect(token.balanceOf(recipient, TOKEN_ID)).toEqual(AMOUNT * 2n); | |
| }); |
🤖 Prompt for AI Agents
In `@contracts/src/token/test/MultiToken.test.ts` around lines 511 - 529, The test
sets approvals for two different operators (Z_SPENDER and Z_OTHER) but both
transfers call ._unsafeTransferFrom via token.as(caller) so Z_OTHER is never
used; update the transfers to perform the first transfer using
token.as(Z_SPENDER)._unsafeTransferFrom(...) and the second transfer using
token.as(Z_OTHER)._unsafeTransferFrom(...) (keeping the existing _mint,
setApprovalForAll, and balanceOf assertions) so the test exercises two distinct
approved operators.
| private readonly options: BuilderOptions; | ||
| private readonly steps: Array<{ cmd: string; msg: string; shell?: string }> = | ||
| [ | ||
| // Step 1: Clean dist directory | ||
| { | ||
| cmd: 'rm -rf dist && mkdir -p dist', | ||
| msg: 'Cleaning dist directory', | ||
| shell: '/bin/bash', | ||
| }, | ||
|
|
||
| // Step 2: TypeScript compilation (witnesses/ -> dist/witnesses/) | ||
| { | ||
| cmd: 'tsc --project tsconfig.build.json', | ||
| msg: 'Compiling TypeScript', | ||
| }, | ||
|
|
||
| // Step 3: Copy .compact files preserving structure (excludes Mock* files and archive/) | ||
| { | ||
| // biome-ignore-start lint/suspicious/noUselessEscapeInString: Needed inside JS template literal | ||
| cmd: ` | ||
| find src -type f -name "*.compact" ! -name "Mock*" ! -path "*/archive/*" | while read file; do | ||
| # Remove src/ prefix from path | ||
| rel_path="\${file#src/}" | ||
| mkdir -p "dist/\$(dirname "\$rel_path")" | ||
| cp "\$file" "dist/\$rel_path" | ||
| done | ||
| `, | ||
| // biome-ignore-end lint/suspicious/noUselessEscapeInString: Needed inside JS template literal | ||
| msg: 'Copying .compact files (excluding mocks and archive)', | ||
| cmd: 'mkdir -p dist/artifacts && cp -Rf src/artifacts/* dist/artifacts/ 2>/dev/null || true', | ||
| msg: 'Copying artifacts', | ||
| shell: '/bin/bash', | ||
| }, | ||
|
|
||
| // Step 4: Copy essential files for distribution | ||
| { | ||
| cmd: ` | ||
| # Copy package.json and README | ||
| cp package.json dist/ 2>/dev/null || true | ||
| cp ../README.md dist/ # Go up one level to monorepo root | ||
| `, | ||
| msg: 'Copying package metadata', | ||
| cmd: 'mkdir -p dist && find src -type f -name "*.compact" -exec cp {} dist/ \\; 2>/dev/null && rm dist/Mock*.compact 2>/dev/null || true', | ||
| msg: 'Copying and cleaning .compact files', | ||
| shell: '/bin/bash', | ||
| }, | ||
| ]; |
There was a problem hiding this comment.
Copy steps ignore srcDir/outDir and can miss compiled artifacts.
The compiler outputs to outDir (default artifacts at repo root), but the build step copies from src/artifacts. This breaks default behavior and any --src/--out usage. Derive the copy paths from options (or reuse the compiler’s resolved directories).
🔧 Suggested fix (derive paths from options)
- private readonly steps: Array<{ cmd: string; msg: string; shell?: string }> =
- [
- {
- cmd: 'tsc --project tsconfig.build.json',
- msg: 'Compiling TypeScript',
- },
- {
- cmd: 'mkdir -p dist/artifacts && cp -Rf src/artifacts/* dist/artifacts/ 2>/dev/null || true',
- msg: 'Copying artifacts',
- shell: '/bin/bash',
- },
- {
- cmd: 'mkdir -p dist && find src -type f -name "*.compact" -exec cp {} dist/ \\; 2>/dev/null && rm dist/Mock*.compact 2>/dev/null || true',
- msg: 'Copying and cleaning .compact files',
- shell: '/bin/bash',
- },
- ];
+ private readonly steps: Array<{ cmd: string; msg: string; shell?: string }>;
+
+ constructor(options: BuilderOptions = {}) {
+ this.options = options;
+ const srcDir = options.srcDir ?? 'src';
+ const outDir = options.outDir ?? 'artifacts';
+ this.steps = [
+ {
+ cmd: 'tsc --project tsconfig.build.json',
+ msg: 'Compiling TypeScript',
+ },
+ {
+ cmd: `mkdir -p dist/artifacts && cp -Rf "${outDir}"/* dist/artifacts/ 2>/dev/null || true`,
+ msg: 'Copying artifacts',
+ shell: '/bin/bash',
+ },
+ {
+ cmd: `mkdir -p dist && find "${srcDir}" -type f -name "*.compact" -exec cp {} dist/ \\; 2>/dev/null && rm dist/Mock*.compact 2>/dev/null || true`,
+ msg: 'Copying and cleaning .compact files',
+ shell: '/bin/bash',
+ },
+ ];
+ }📝 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.
| private readonly options: BuilderOptions; | |
| private readonly steps: Array<{ cmd: string; msg: string; shell?: string }> = | |
| [ | |
| // Step 1: Clean dist directory | |
| { | |
| cmd: 'rm -rf dist && mkdir -p dist', | |
| msg: 'Cleaning dist directory', | |
| shell: '/bin/bash', | |
| }, | |
| // Step 2: TypeScript compilation (witnesses/ -> dist/witnesses/) | |
| { | |
| cmd: 'tsc --project tsconfig.build.json', | |
| msg: 'Compiling TypeScript', | |
| }, | |
| // Step 3: Copy .compact files preserving structure (excludes Mock* files and archive/) | |
| { | |
| // biome-ignore-start lint/suspicious/noUselessEscapeInString: Needed inside JS template literal | |
| cmd: ` | |
| find src -type f -name "*.compact" ! -name "Mock*" ! -path "*/archive/*" | while read file; do | |
| # Remove src/ prefix from path | |
| rel_path="\${file#src/}" | |
| mkdir -p "dist/\$(dirname "\$rel_path")" | |
| cp "\$file" "dist/\$rel_path" | |
| done | |
| `, | |
| // biome-ignore-end lint/suspicious/noUselessEscapeInString: Needed inside JS template literal | |
| msg: 'Copying .compact files (excluding mocks and archive)', | |
| cmd: 'mkdir -p dist/artifacts && cp -Rf src/artifacts/* dist/artifacts/ 2>/dev/null || true', | |
| msg: 'Copying artifacts', | |
| shell: '/bin/bash', | |
| }, | |
| // Step 4: Copy essential files for distribution | |
| { | |
| cmd: ` | |
| # Copy package.json and README | |
| cp package.json dist/ 2>/dev/null || true | |
| cp ../README.md dist/ # Go up one level to monorepo root | |
| `, | |
| msg: 'Copying package metadata', | |
| cmd: 'mkdir -p dist && find src -type f -name "*.compact" -exec cp {} dist/ \\; 2>/dev/null && rm dist/Mock*.compact 2>/dev/null || true', | |
| msg: 'Copying and cleaning .compact files', | |
| shell: '/bin/bash', | |
| }, | |
| ]; | |
| private readonly options: BuilderOptions; | |
| private readonly steps: Array<{ cmd: string; msg: string; shell?: string }>; | |
| constructor(options: BuilderOptions = {}) { | |
| this.options = options; | |
| const srcDir = options.srcDir ?? 'src'; | |
| const outDir = options.outDir ?? 'artifacts'; | |
| this.steps = [ | |
| { | |
| cmd: 'tsc --project tsconfig.build.json', | |
| msg: 'Compiling TypeScript', | |
| }, | |
| { | |
| cmd: `mkdir -p dist/artifacts && cp -Rf "${outDir}"/* dist/artifacts/ 2>/dev/null || true`, | |
| msg: 'Copying artifacts', | |
| shell: '/bin/bash', | |
| }, | |
| { | |
| cmd: `mkdir -p dist && find "${srcDir}" -type f -name "*.compact" -exec cp {} dist/ \\; 2>/dev/null && rm dist/Mock*.compact 2>/dev/null || true`, | |
| msg: 'Copying and cleaning .compact files', | |
| shell: '/bin/bash', | |
| }, | |
| ]; | |
| } |
🤖 Prompt for AI Agents
In `@packages/compact/src/Builder.ts` around lines 66 - 83, The build steps in
Builder (the steps array) hardcode paths like "src/artifacts" and "dist",
causing mismatch with configured src/out directories; update Builder to derive
those copy/move commands from the instance options (e.g., use options.srcDir and
options.outDir or the compiler-resolved directories) and replace the hardcoded
paths in the steps entries for "Copying artifacts" and "Copying and cleaning
.compact files" so the shell commands reference the resolved src/out locations;
ensure the mkdir/cp/find commands use the derived paths and preserve the
existing shell flags and fallbacks.
| * Truncates to 30 bytes before prepending PREFIX_ADDRESS to comply | ||
| * with field value constraints (max unsigned integer is 2^248-1). | ||
| * The truncation compensates for the bytes added by the prefix. | ||
| * @param str String to hexify and encode. | ||
| * @returns Encoded `ZswapCoinPublicKey`. | ||
| * @returns Encoded `Compact.ContractAddress`. | ||
| */ | ||
| export const encodeToAddress = (str: string): Compact.ContractAddress => ({ | ||
| bytes: encodeContractAddress(PREFIX_ADDRESS + toHexPadded(str)), | ||
| }); | ||
| export const encodeToAddress = (str: string): Compact.ContractAddress => { | ||
| const hex = toHexPadded(str); | ||
| const truncated = hex.slice(0, -4); | ||
| return { | ||
| bytes: encodeContractAddress(PREFIX_ADDRESS + truncated), | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find and read the address.ts file to understand the context
cat -n packages/simulator/test/fixtures/utils/address.tsRepository: OpenZeppelin/compact-contracts
Length of output: 4290
🏁 Script executed:
# Search for toHexPadded function definition
rg "toHexPadded" --type ts -A 5Repository: OpenZeppelin/compact-contracts
Length of output: 15519
🏁 Script executed:
# Search for PREFIX_ADDRESS definition
rg "PREFIX_ADDRESS" --type ts -B 2 -A 2Repository: OpenZeppelin/compact-contracts
Length of output: 2737
🏁 Script executed:
# Search for encodeContractAddress function
rg "encodeContractAddress" --type ts -B 2 -A 5Repository: OpenZeppelin/compact-contracts
Length of output: 7915
🌐 Web query:
@midnight-ntwrk/ledger-v7 encodeContractAddress contract address payload length specification
💡 Result:
In @midnight-ntwrk/ledger-v7, encodeContractAddress(...) expects the raw contract address payload to be 32 bytes (256 bits) — i.e. a 64-hex-character value (often written as 0x + 64 hex chars). This matches how Midnight JSON-RPC APIs represent contract addresses (32-byte hex). [1]
If you’re encoding from bytes, the input must therefore be exactly 32 bytes; if you’re encoding from hex, it must be exactly 64 hex chars (optionally with a 0x prefix). The returned string is the Bech32m-encoded form used across the Midnight wallet ecosystem. [2]
Sources:
[1] https://www.ankr.com/docs/rpc-service/chains/chains-api/midnight/
[2] https://www.npmjs.com/package/%40midnight-ntwrk%2Fwallet-sdk-address-format
🏁 Script executed:
# Check how encodeToAddress is called in the codebase
rg "encodeToAddress" --type ts -B 2 -A 2Repository: OpenZeppelin/compact-contracts
Length of output: 9142
🏁 Script executed:
# Check what strings are passed to encodeToAddress
rg "encodeToAddress\(" --type ts -A 1 | head -50Repository: OpenZeppelin/compact-contracts
Length of output: 2283
The current logic correctly produces 64 hex characters for typical test inputs, but lacks explicit input clamping for edge cases.
For normal short strings (current usage), toHexPadded(str) → 64 hex chars (32 bytes) → slice(0, -4) → 60 hex chars (30 bytes) → PREFIX_ADDRESS + truncated → 64 hex chars (32 bytes), which matches the encodeContractAddress requirement. However, if a long ASCII string (>32 bytes) is passed, toHexPadded produces >64 hex characters, and the slice only removes 4 chars, risking oversized payloads. Consider adding explicit clamping:
Suggested improvement (defensive clamping)
- const hex = toHexPadded(str);
- const truncated = hex.slice(0, -4);
+ const hex = toHexPadded(str).slice(0, 64);
+ const truncated = hex.slice(0, -4);📝 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.
| * Truncates to 30 bytes before prepending PREFIX_ADDRESS to comply | |
| * with field value constraints (max unsigned integer is 2^248-1). | |
| * The truncation compensates for the bytes added by the prefix. | |
| * @param str String to hexify and encode. | |
| * @returns Encoded `ZswapCoinPublicKey`. | |
| * @returns Encoded `Compact.ContractAddress`. | |
| */ | |
| export const encodeToAddress = (str: string): Compact.ContractAddress => ({ | |
| bytes: encodeContractAddress(PREFIX_ADDRESS + toHexPadded(str)), | |
| }); | |
| export const encodeToAddress = (str: string): Compact.ContractAddress => { | |
| const hex = toHexPadded(str); | |
| const truncated = hex.slice(0, -4); | |
| return { | |
| bytes: encodeContractAddress(PREFIX_ADDRESS + truncated), | |
| }; | |
| * Truncates to 30 bytes before prepending PREFIX_ADDRESS to comply | |
| * with field value constraints (max unsigned integer is 2^248-1). | |
| * The truncation compensates for the bytes added by the prefix. | |
| * `@param` str String to hexify and encode. | |
| * `@returns` Encoded `Compact.ContractAddress`. | |
| */ | |
| export const encodeToAddress = (str: string): Compact.ContractAddress => { | |
| const hex = toHexPadded(str).slice(0, 64); | |
| const truncated = hex.slice(0, -4); | |
| return { | |
| bytes: encodeContractAddress(PREFIX_ADDRESS + truncated), | |
| }; |
🤖 Prompt for AI Agents
In `@packages/simulator/test/fixtures/utils/address.ts` around lines 32 - 43,
encodeToAddress may produce oversized hex when toHexPadded(str) returns >64
chars; ensure you clamp the payload explicitly before prefixing: compute hex =
toHexPadded(str), then truncate/clamp hex to at most 60 hex characters (30
bytes) for the part that you will prepend PREFIX_ADDRESS to, so that
PREFIX_ADDRESS + truncated is always 64 hex chars; keep using
encodeContractAddress on the resulting fixed-length string and reference
encodeToAddress, toHexPadded, PREFIX_ADDRESS, and encodeContractAddress when
making the change.
| function isSpawnSyncRet( | ||
| err: unknown, | ||
| ): err is SpawnSyncReturns<string | Buffer> { | ||
| if (typeof err !== 'object' || err === null) { | ||
| return false; | ||
| } | ||
|
|
||
| const typedErr = err as Partial<SpawnSyncReturns<string | Buffer>> & | ||
| Record<string, unknown>; | ||
|
|
||
| const okErr = typedErr.error instanceof Error; | ||
| const okStdout = | ||
| typeof typedErr.stdout === 'string' || Buffer.isBuffer(typedErr.stdout); | ||
| const okStderr = | ||
| typeof typedErr.stderr === 'string' || Buffer.isBuffer(typedErr.stderr); | ||
| const okStatus = | ||
| typeof typedErr.status === 'number' || typedErr.status === null; | ||
|
|
||
| return okErr && okStdout && okStderr && okStatus; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, verify the error shape from promisified exec
cat << 'EOF' | node --input-type=module
import { exec } from 'node:child_process';
import { promisify } from 'node:util';
const execAsync = promisify(exec);
try {
await execAsync('nonexistent-command-12345');
} catch (e) {
console.log('Error keys:', Object.keys(e));
console.log('Has status:', 'status' in e);
console.log('Has code:', 'code' in e);
console.log('code value:', e.code);
console.log('status value:', e.status);
console.log('Full error type:', e.constructor.name);
}
EOFRepository: OpenZeppelin/compact-contracts
Length of output: 239
🏁 Script executed:
# Find the setup.ts file and examine it
fd -t f "setup.ts" packages/simulator/test/Repository: OpenZeppelin/compact-contracts
Length of output: 106
🏁 Script executed:
# Search for how isSpawnSyncRet is used in the codebase
rg "isSpawnSyncRet" -A 3 -B 3Repository: OpenZeppelin/compact-contracts
Length of output: 827
🏁 Script executed:
# Check if promisify(exec) is used in the setup.ts file
rg "promisify|exec" packages/simulator/test/setup.ts -A 2 -B 2Repository: OpenZeppelin/compact-contracts
Length of output: 592
Type guard for SpawnSyncReturns does not match promisified exec errors.
The isSpawnSyncRet function checks for properties specific to SpawnSyncReturns: error, status, stdout, and stderr. However, promisify(exec) rejects with an Error that has only code, killed, signal, cmd, stdout, and stderr properties—no error or status property.
Since the guard checks typedErr.error instanceof Error and typedErr.status (as number or null), both will fail for exec errors, causing the function to return false and the error to be re-thrown on line 51 rather than being handled.
🤖 Prompt for AI Agents
In `@packages/simulator/test/setup.ts` around lines 26 - 45, The current
isSpawnSyncRet type guard requires SpawnSyncReturns-specific fields (error,
status) which prometified exec errors don't have; update isSpawnSyncRet to
return true for either the SpawnSyncReturns shape or the exec-style error shape
by checking stdout/stderr (string|Buffer) plus one of: (typedErr.error
instanceof Error) OR exec-style properties like typeof typedErr.code ===
'number' || typeof typedErr.code === 'string' || typedErr.code === null, typeof
typedErr.killed === 'boolean', and (typeof typedErr.signal === 'string' ||
typedErr.signal === null) and optionally typeof typedErr.cmd === 'string'; use
these symbol names (isSpawnSyncRet, SpawnSyncReturns, promisify(exec)) to locate
and replace the guard logic so exec rejection objects are recognized and handled
instead of re-thrown.
| const modifiedTxCtx: QueryContext = { | ||
| ...ctx.transactionContext, | ||
| ...ctx.currentQueryContext, | ||
| address: encodeToAddress('otherAddress'), | ||
| } as unknown as QueryContext; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find and examine the StateManager test file around lines 122-125
cat -n packages/simulator/test/unit/core/StateManager.test.ts | sed -n '110,140p'Repository: OpenZeppelin/compact-contracts
Length of output: 1230
🏁 Script executed:
# Search for QueryContext definition
rg "interface QueryContext|type QueryContext|class QueryContext" -A 5Repository: OpenZeppelin/compact-contracts
Length of output: 56
🏁 Script executed:
# Search for QueryContext-related methods
rg "QueryContext" --type ts -A 2 | grep -E "(clone|copy|create|factory|make)" -iRepository: OpenZeppelin/compact-contracts
Length of output: 764
🏁 Script executed:
# Search for QueryContext usage patterns
rg "currentQueryContext\|QueryContext.*\{" --type ts -B 2 -A 2 | head -100Repository: OpenZeppelin/compact-contracts
Length of output: 56
🏁 Script executed:
# Search for QueryContext in all files, including imports
rg "QueryContext" --type ts | head -50Repository: OpenZeppelin/compact-contracts
Length of output: 3093
🏁 Script executed:
# Get imports and definitions from the test file
head -50 packages/simulator/test/unit/core/StateManager.test.tsRepository: OpenZeppelin/compact-contracts
Length of output: 1602
🏁 Script executed:
# Find where QueryContext might be defined
fd QueryContext --type tsRepository: OpenZeppelin/compact-contracts
Length of output: 243
🏁 Script executed:
# Look for methods on QueryContext or similar patterns
rg "withAddress|clone|copy" --type ts -B 2 -A 3 | head -100Repository: OpenZeppelin/compact-contracts
Length of output: 601
🏁 Script executed:
# Search for QueryContext constructor definition
rg "new QueryContext\(" --type ts -B 2 -A 2Repository: OpenZeppelin/compact-contracts
Length of output: 2733
🏁 Script executed:
# Look for QueryContext in node_modules or package dependencies
rg "class QueryContext|export.*QueryContext" --type ts -A 5Repository: OpenZeppelin/compact-contracts
Length of output: 56
🏁 Script executed:
# Get more context around the problematic code and similar patterns
cat -n packages/simulator/test/unit/core/StateManager.test.ts | sed -n '100,160p'Repository: OpenZeppelin/compact-contracts
Length of output: 1644
🏁 Script executed:
# Look at how the state property is accessed and used
rg "currentQueryContext\.state" --type ts -B 1 -A 1 | head -50Repository: OpenZeppelin/compact-contracts
Length of output: 1396
🏁 Script executed:
# Search for other test patterns with QueryContext
rg "QueryContext.*\{" --type ts -B 2 -A 5Repository: OpenZeppelin/compact-contracts
Length of output: 723
🏁 Script executed:
# Look for comments or patterns around QueryContext creation
rg "QueryContext" packages/simulator/src/core/CircuitContextManager.ts -B 3 -A 3Repository: OpenZeppelin/compact-contracts
Length of output: 423
Use QueryContext constructor instead of spreading. The spread+cast pattern loses the WASM backing (__wbg_ptr). Use new QueryContext(ctx.currentQueryContext.state, encodeToAddress('otherAddress')) to properly clone with the updated address.
🤖 Prompt for AI Agents
In `@packages/simulator/test/unit/core/StateManager.test.ts` around lines 122 -
125, Replace the spread+cast pattern that loses WASM backing by constructing a
proper QueryContext instance: instead of creating modifiedTxCtx with
{...ctx.currentQueryContext, address: encodeToAddress('otherAddress')} as
QueryContext, call the QueryContext constructor with the original state and the
new address (use ctx.currentQueryContext.state and
encodeToAddress('otherAddress')) so the resulting object retains the WASM
__wbg_ptr backing.
andrew-fleming
left a comment
andrew-fleming
left a comment
There was a problem hiding this comment.
Looking good, @emnul! I left some comments
This is 98% reviewed, I'll finish the nft and multitoken modules + tests tomorrow
| type ZswapCoinPublicKey = { bytes: Uint8Array }; | ||
|
|
||
| type ContractAddress = { bytes: Uint8Array }; | ||
|
|
||
| type Either<A, B> = { is_left: boolean; left: A; right: B }; |
There was a problem hiding this comment.
This also works :) I'd lean toward importing instead of duplicating the types down the road though once we have a nice solution
There was a problem hiding this comment.
Totally agree, should we open an issue in the ledger repo and propose to expose these Ledger ADT types the on-chain runtime or ledger package?
There was a problem hiding this comment.
I think that's a good idea. It's a small dx improvement but everyone benefits. Worst case, it's something we can offer
There was a problem hiding this comment.
Similar question from the build changes: Is this necessary to bump the version?
0xisk
left a comment
0xisk
left a comment
There was a problem hiding this comment.
Thank you @emnul! I just have a concern about the changes on the packages/compact, we want at the end to switch to use compact-tools. I see we don't want to block this PR until the compact-tools gets released, but I'd say let's open an issue on compact-tools for the changes happened here.
Co-authored-by: Andrew Fleming <fleming-andrew@protonmail.com> Signed-off-by: ⟣ €₥ℵ∪ℓ ⟢ <34749913+emnul@users.noreply.github.com>
Co-authored-by: Andrew Fleming <fleming-andrew@protonmail.com> Signed-off-by: ⟣ €₥ℵ∪ℓ ⟢ <34749913+emnul@users.noreply.github.com>
|
@andrew-fleming RE: Compact tools changes Really the only change that's 100% necessary is the archive directory filtering in Should those sync changes be reverted for now? |
@emnul aint no thing. To be fair, we haven't been able to publish 😢 now, I'm wondering if we should be running build for contracts from the tools package bc it's pretty specific for how we use it
I'd say yeah, if you don't mind. Let's revert those changes to keep this PR clean so we're not yoloing anything. Measure twice, cut once |
|
|
||
| expect(() => { | ||
| token.transferFrom(Z_OWNER, Z_RECIPIENT, 1n, caller); | ||
| token.as(OWNER).transferFrom(Z_OWNER, Z_RECIPIENT, 1n); |
There was a problem hiding this comment.
| token.as(OWNER).transferFrom(Z_OWNER, Z_RECIPIENT, 1n); | |
| token.as(SPENDER).transferFrom(Z_OWNER, Z_RECIPIENT, 1n); |
| type ZswapCoinPublicKey = { bytes: Uint8Array }; | ||
|
|
||
| type ContractAddress = { bytes: Uint8Array }; | ||
|
|
||
| type Either<A, B> = { is_left: boolean; left: A; right: B }; |
There was a problem hiding this comment.
I think that's a good idea. It's a small dx improvement but everyone benefits. Worst case, it's something we can offer
| token.as(caller).setApprovalForAll(Z_SPENDER, true); | ||
| token.as(caller).setApprovalForAll(Z_OTHER, true); |
There was a problem hiding this comment.
We should probably move this out of the 'transferFrom', When the caller is - block
| token.as(caller).setApprovalForAll(Z_SPENDER, true); | |
| token.as(caller).setApprovalForAll(Z_OTHER, true); | |
| token.as(OWNER).setApprovalForAll(Z_SPENDER, true); | |
| token.as(OWNER).setApprovalForAll(Z_OTHER, true); |
| token | ||
| .as(SPENDER) | ||
| .transferFrom(Z_OWNER, Z_RECIPIENT, TOKEN_ID, AMOUNT); |
There was a problem hiding this comment.
| token | |
| .as(SPENDER) | |
| .transferFrom(Z_OWNER, Z_RECIPIENT, TOKEN_ID, AMOUNT); | |
| token | |
| .as(OTHER) | |
| .transferFrom(Z_OWNER, Z_RECIPIENT, TOKEN_ID, AMOUNT); |
If the above suggestion is accepted, this should be OTHER to follow this test's goal (also need to create OTHER in the pk gen)
| token | ||
| .as(caller) | ||
| ._unsafeTransferFrom(Z_OWNER, recipient, TOKEN_ID, AMOUNT); |
There was a problem hiding this comment.
| token | |
| .as(caller) | |
| ._unsafeTransferFrom(Z_OWNER, recipient, TOKEN_ID, AMOUNT); | |
| token | |
| .as(OTHER) | |
| ._unsafeTransferFrom(Z_OWNER, recipient, TOKEN_ID, AMOUNT); |
Similar thing here
| .as(caller) | ||
| ._unsafeTransferFrom(Z_OWNER, recipient, TOKEN_ID, AMOUNT); |
There was a problem hiding this comment.
| .as(caller) | |
| ._unsafeTransferFrom(Z_OWNER, recipient, TOKEN_ID, AMOUNT); | |
| .as(SPENDER) | |
| ._unsafeTransferFrom(Z_OWNER, recipient, TOKEN_ID, AMOUNT); |
| token.as(SPENDER).transferFrom(Z_OWNER, Z_SPENDER, TOKENID_1); | ||
| token.as(SPENDER).transferFrom(Z_OWNER, Z_SPENDER, TOKENID_2); | ||
| token.as(SPENDER).transferFrom(Z_OWNER, Z_SPENDER, TOKENID_3); |
There was a problem hiding this comment.
| token.as(SPENDER).transferFrom(Z_OWNER, Z_SPENDER, TOKENID_1); | |
| token.as(SPENDER).transferFrom(Z_OWNER, Z_SPENDER, TOKENID_2); | |
| token.as(SPENDER).transferFrom(Z_OWNER, Z_SPENDER, TOKENID_3); | |
| token.setPersistentCaller(SPENDER); | |
| token.transferFrom(Z_OWNER, Z_SPENDER, TOKENID_1); | |
| token.transferFrom(Z_OWNER, Z_SPENDER, TOKENID_2); | |
| token.transferFrom(Z_OWNER, Z_SPENDER, TOKENID_3); |
Not opposed to keeping the caller ctx explicit for each call. This is just another way to do it
3 participants
This PR is part of #364 .
fromvariables tofromAddressto avoid new keyword collisions.compact fixup)archivecontracts. It should be removed in Add new compiler flag to skip directories #367createSimulatorfactory functionaddress.tsfiles.as(ROLE)syntaxencodeToAddressto catch changes toContractAddressrequirements and meet new type expectationsarchivecontracts from testsSummary by CodeRabbit
New Features
Bug Fixes
Chores