From 0d2d8c20b62ec7aa7b272939ae73cb19622f8f99 Mon Sep 17 00:00:00 2001 From: Rosco Kalis Date: Tue, 29 Sep 2026 11:29:42 +0200 Subject: [PATCH] fix: fix init-reassign for-loop bug --- .../src/generation/GenerateTargetTraversal.ts | 6 ++ .../for_loop_reassigned_init.ts | 39 ++++++++ ...lobal_function_for_loop_reassigned_init.ts | 90 +++++++++++++++++++ .../for_loop_reassigned_init.cash | 11 +++ ...bal_function_for_loop_reassigned_init.cash | 15 ++++ website/docs/releases/release-notes.md | 1 + 6 files changed, 162 insertions(+) create mode 100644 packages/cashc/test/generation/fixtures/valid-contract-files/for_loop_reassigned_init.ts create mode 100644 packages/cashc/test/generation/fixtures/valid-contract-files/global_function_for_loop_reassigned_init.ts create mode 100644 packages/cashc/test/valid-contract-files/for_loop_reassigned_init.cash create mode 100644 packages/cashc/test/valid-contract-files/global_function_for_loop_reassigned_init.cash diff --git a/packages/cashc/src/generation/GenerateTargetTraversal.ts b/packages/cashc/src/generation/GenerateTargetTraversal.ts index df6f3cb92..e0f23d785 100644 --- a/packages/cashc/src/generation/GenerateTargetTraversal.ts +++ b/packages/cashc/src/generation/GenerateTargetTraversal.ts @@ -751,7 +751,13 @@ export default class GenerateTargetTraversal extends AstTraversal { visitFor(node: ForNode): Node { const forScopeStackDepth = this.stack.length; + + // If the init is a re-assignment, we need to increment the scope depth so it gets treated as a scoped reassignment + // (and therefore the stack value gets properly replaced) + const isReassignment = node.init instanceof AssignNode; + if (isReassignment) this.scopeDepth += 1; node.init = this.visit(node.init) as VariableDefinitionNode | AssignNode; + if (isReassignment) this.scopeDepth -= 1; this.scopeDepth += 1; this.emit(Op.OP_BEGIN, { location: node.location, positionHint: PositionHint.START }); diff --git a/packages/cashc/test/generation/fixtures/valid-contract-files/for_loop_reassigned_init.ts b/packages/cashc/test/generation/fixtures/valid-contract-files/for_loop_reassigned_init.ts new file mode 100644 index 000000000..f463442da --- /dev/null +++ b/packages/cashc/test/generation/fixtures/valid-contract-files/for_loop_reassigned_init.ts @@ -0,0 +1,39 @@ +import { Fixture } from '../../fixture-utils.js'; + +export const fixtures: Fixture[] = [ + { + // A for-loop init that reassigns an existing variable replaces that variable's stack slot, so the loop's + // final value is what the require after the loop reads (the loop's cleanup does not drop it) + artifact: { + contractName: 'ForLoopReassignedInit', + constructorInputs: [], + abi: [{ name: 'spend', inputs: [{ name: 'n', type: 'int' }] }], + bytecode: + // int i = 0; + 'OP_0 ' + // for (i = 0; i < n; i++) { + + 'OP_DROP OP_0 OP_BEGIN OP_2DUP OP_GREATERTHAN OP_DUP OP_TOALTSTACK OP_IF ' + // require(tx.outputs[i].value >= 1000); + + 'OP_DUP OP_OUTPUTVALUE e803 OP_GREATERTHANOREQUAL OP_VERIFY ' + // For loop update + + 'OP_1ADD ' + // Loop condition + + 'OP_ENDIF OP_FROMALTSTACK OP_NOT OP_UNTIL ' + // require(i == tx.outputs.length); + + 'OP_TXOUTPUTCOUNT OP_NUMEQUAL ' + // Cleanup + + 'OP_NIP', + fingerprint: 'efe2107d45ab15018a2d5a2bc2cb0311d86ec7c8ba5e01c8503f25619ecec621', + debug: { + bytecode: '007500656ea0766b6376cc02e803a2698b686c9166c49c77', + sourceMap: '3:16:3:17;5:13:5:18:1;;:8:7:9:0;:20:5:25;::::1;;;:32:7:9:0;6:31:6:32;:20::39:1;:43::47:0;:20:::1;:12::49;5:27:5:30;:32:7:9;;:8;;9:21:9:38:0;:8::40:1;2:26:10:5', + logs: [], + requires: [ + { ip: 13, line: 6 }, + { ip: 21, line: 9 }, + ], + sourceTags: '14:14:fu;15:18:lc;21:21:sc', + }, + }, + }, +]; diff --git a/packages/cashc/test/generation/fixtures/valid-contract-files/global_function_for_loop_reassigned_init.ts b/packages/cashc/test/generation/fixtures/valid-contract-files/global_function_for_loop_reassigned_init.ts new file mode 100644 index 000000000..be578b3cc --- /dev/null +++ b/packages/cashc/test/generation/fixtures/valid-contract-files/global_function_for_loop_reassigned_init.ts @@ -0,0 +1,90 @@ +import { Fixture } from '../../fixture-utils.js'; + +export const fixtures: Fixture[] = [ + { + // A global function returning a variable that a for-loop init reassigned returns the loop's final value + artifact: { + contractName: 'GlobalFunctionForLoopReassignedInit', + constructorInputs: [], + abi: [{ name: 'spend', inputs: [{ name: 'n', type: 'int' }] }], + bytecode: + // require(count(n) == n), with count(n) spliced in: + 'OP_DUP ' + // int i = 0; + + 'OP_0 ' + // for (i = 0; i < n; i++) { + + 'OP_DROP OP_0 OP_BEGIN OP_2DUP OP_GREATERTHAN OP_DUP OP_TOALTSTACK OP_IF ' + // require(i < 10); + + 'OP_DUP OP_10 OP_LESSTHAN OP_VERIFY ' + // For loop update + + 'OP_1ADD ' + // Loop condition + + 'OP_ENDIF OP_FROMALTSTACK OP_NOT OP_UNTIL ' + // return i; + + 'OP_NIP ' + // == n + + 'OP_NUMEQUAL', + fingerprint: '9643acfe0e75c3bfacc39e2ff86267dc6d707f341818b2c3a83795038141d11e', + debug: { + bytecode: '76007500656ea0766b63765a9f698b686c9166779c', + sourceMap: '13:22:13:23;:16::24:1;;;;;;;;;;;;;;;;;;;:8::31', + logs: [], + requires: [ + { ip: 13, line: 13 }, + { ip: 21, line: 13 }, + ], + sourceTags: '14:14:fu;15:18:lc;19:19:sc', + functions: [ + { + name: 'count', + inputs: [{ name: 'n', type: 'int' }], + bytecode: '007500656ea0766b63765a9f698b686c916677', + sourceMap: '2:12:2:13;4:9:4:14:1;;:4:6:5:0;:16:4:21;::::1;;;:28:6:5:0;5:16:5:17;:20::22;:16:::1;:8::24;4:23:4:26;:28:6:5;;:4;;1:36:9:1', + sourceTags: '13:13:fu;14:17:lc;18:18:sc', + logs: [], + requires: [ + { ip: 12, line: 5 }, + ], + }, + ], + inlineRanges: '1:19:count', + }, + }, + }, + { + compilerOptions: { disableInlining: true }, + artifact: { + contractName: 'GlobalFunctionForLoopReassignedInit', + constructorInputs: [], + abi: [{ name: 'spend', inputs: [{ name: 'n', type: 'int' }] }], + bytecode: + // OP_DEFINE count (id 0): the same body as the inlined variant, ending in return i (OP_NIP) + '007500656ea0766b63765a9f698b686c916677 OP_0 OP_DEFINE ' + // require(count(n) == n) + + 'OP_DUP OP_0 OP_INVOKE OP_NUMEQUAL', + fingerprint: '33bcf111dab3f5359532cf1f0d54027b49f5f7a708614569d905143b953894f4', + debug: { + bytecode: '13007500656ea0766b63765a9f698b686c916677008976008a9c', + sourceMap: '1::9:1;;::::1;13:22:13:23:0;:16::24:1;;:8::31', + logs: [], + requires: [ + { ip: 7, line: 13 }, + ], + functions: [ + { + id: 0, + name: 'count', + inputs: [{ name: 'n', type: 'int' }], + bytecode: '007500656ea0766b63765a9f698b686c916677', + sourceMap: '2:12:2:13;4:9:4:14:1;;:4:6:5:0;:16:4:21;::::1;;;:28:6:5:0;5:16:5:17;:20::22;:16:::1;:8::24;4:23:4:26;:28:6:5;;:4;;1:36:9:1', + sourceTags: '13:13:fu;14:17:lc;18:18:sc', + logs: [], + requires: [ + { ip: 12, line: 5 }, + ], + }, + ], + }, + }, + }, +]; diff --git a/packages/cashc/test/valid-contract-files/for_loop_reassigned_init.cash b/packages/cashc/test/valid-contract-files/for_loop_reassigned_init.cash new file mode 100644 index 000000000..44fe97e1b --- /dev/null +++ b/packages/cashc/test/valid-contract-files/for_loop_reassigned_init.cash @@ -0,0 +1,11 @@ +contract ForLoopReassignedInit() { + function spend(int n) { + int i = 0; + + for (i = 0; i < n; i++) { + require(tx.outputs[i].value >= 1000); + } + + require(i == tx.outputs.length); + } +} diff --git a/packages/cashc/test/valid-contract-files/global_function_for_loop_reassigned_init.cash b/packages/cashc/test/valid-contract-files/global_function_for_loop_reassigned_init.cash new file mode 100644 index 000000000..07bcd9037 --- /dev/null +++ b/packages/cashc/test/valid-contract-files/global_function_for_loop_reassigned_init.cash @@ -0,0 +1,15 @@ +function count(int n) returns (int) { + int i = 0; + + for (i = 0; i < n; i++) { + require(i < 10); + } + + return i; +} + +contract GlobalFunctionForLoopReassignedInit() { + function spend(int n) { + require(count(n) == n); + } +} diff --git a/website/docs/releases/release-notes.md b/website/docs/releases/release-notes.md index eeb8a7f37..d2e589802 100644 --- a/website/docs/releases/release-notes.md +++ b/website/docs/releases/release-notes.md @@ -18,6 +18,7 @@ This release contains several breaking changes, please refer to the [migration n - :bug: Fix bug where date literal parsing was different per locale, it now uses UTC. - :bug: Fix bug where `LockingBytecodeNullData` used an incorrect push opcode for empty chunks and chunks of 128-255 bytes, so it did not match the SDK's `addOpReturnOutput()`. Chunks with a value or length that is known at compile time now use a precomputed push opcode, which makes them smaller. This changes the bytecode of all contracts that use `LockingBytecodeNullData`. - :bug: Fix bug where hex literals with an odd number of digits (e.g. `0x123`) compiled to a different value (`0x1203`), they now cause a compile error. +- :bug: Fix bug where a `for` loop whose init reassigns an existing variable did not update the stack value after the loop. - :racehorse: Add new `OP_SWAP OP_MUL`, `OP_NOT OP_NOT` and `OP_TOALTSTACK OP_FROMALTSTACK` optimisations. - :racehorse: Add new optimisations for loop counter updates, reassignments inside loops and order-independent operations (`OP_BOOLAND`, `OP_BOOLOR`, `OP_MIN`, `OP_MAX`). - :racehorse: Greatly improve compiler speed for very large contracts.