From 9b003bdbfb0d1dfe3e12977a77b945640d8ad616 Mon Sep 17 00:00:00 2001 From: Anton Trunov Date: Wed, 24 Jul 2024 08:24:41 +0400 Subject: [PATCH] fix(codegen): allocator not accounting for extra ref in a cell resulting in cell overflows in some cases (#615) resulting in cell overflows in some cases --- CHANGELOG.md | 1 + examples/__snapshots__/increment.spec.ts.snap | 12 +++--- .../writeSerialization.spec.ts.snap | 4 +- .../resolveAllocation.spec.ts.snap | 32 ++++++++-------- src/storage/allocator.ts | 21 ++++++++--- .../__snapshots__/intrinsics.spec.ts.snap | 2 +- .../__snapshots__/map.spec.ts.snap | 6 +-- src/test/e2e-emulated/allocation.spec.ts | 32 ++++++++++++++++ .../e2e-emulated/contracts/allocation.tact | 37 +++++++++++++++++++ tact.config.json | 5 +++ 10 files changed, 119 insertions(+), 33 deletions(-) create mode 100644 src/test/e2e-emulated/allocation.spec.ts create mode 100644 src/test/e2e-emulated/contracts/allocation.tact diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b4a17582..1428b3655 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,6 +50,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Defining non-existing native FunC functions now throws an understandable compilation error: PR [#585](https://github.com/tact-lang/tact/pull/585) - Bump used `@tact-lang/opcode` version to `0.0.16` which fixes the issue with `DIV` instructions: PR [#589](https://github.com/tact-lang/tact/pull/589) - Incorrect FunC code generated for `recv_external`: PR [#604](https://github.com/tact-lang/tact/pull/604) +- Allocator bug resulting in cell overflows for some contract data layouts: PR [#615](https://github.com/tact-lang/tact/pull/615) ## [1.4.0] - 2024-06-21 diff --git a/examples/__snapshots__/increment.spec.ts.snap b/examples/__snapshots__/increment.spec.ts.snap index 1bbf4e4ab..3cc372a94 100644 --- a/examples/__snapshots__/increment.spec.ts.snap +++ b/examples/__snapshots__/increment.spec.ts.snap @@ -20,14 +20,14 @@ exports[`increment should deploy 1`] = ` }, "bounce": true, "from": "@treasure(treasure)", - "to": "kQCFVwtFcoSZ-muVDfXKSpI0fzDrWMuzTgXqo4_QDKKycmXS", + "to": "kQCZq6o-nSHCoxfIzdCAl3fDePr-LEYT-VUlqbQ5prvrJ7Y2", "type": "internal", "value": "10", }, }, { "$type": "processed", - "gasUsed": 8491n, + "gasUsed": 8473n, }, { "$type": "sent", @@ -41,10 +41,10 @@ exports[`increment should deploy 1`] = ` }, }, "bounce": false, - "from": "kQCFVwtFcoSZ-muVDfXKSpI0fzDrWMuzTgXqo4_QDKKycmXS", + "from": "kQCZq6o-nSHCoxfIzdCAl3fDePr-LEYT-VUlqbQ5prvrJ7Y2", "to": "@treasure(treasure)", "type": "internal", - "value": "9.990313", + "value": "9.990331", }, ], }, @@ -71,14 +71,14 @@ exports[`increment should deploy 2`] = ` }, "bounce": true, "from": "@treasure(treasure)", - "to": "kQCFVwtFcoSZ-muVDfXKSpI0fzDrWMuzTgXqo4_QDKKycmXS", + "to": "kQCZq6o-nSHCoxfIzdCAl3fDePr-LEYT-VUlqbQ5prvrJ7Y2", "type": "internal", "value": "10", }, }, { "$type": "processed", - "gasUsed": 6007n, + "gasUsed": 5963n, }, ], }, diff --git a/src/generator/writers/__snapshots__/writeSerialization.spec.ts.snap b/src/generator/writers/__snapshots__/writeSerialization.spec.ts.snap index a38e2e511..7f9f74a1f 100644 --- a/src/generator/writers/__snapshots__/writeSerialization.spec.ts.snap +++ b/src/generator/writers/__snapshots__/writeSerialization.spec.ts.snap @@ -13007,8 +13007,8 @@ return result;", "code": "var (v'a, v'b, v'c, v'd, v'e, v'f, v'g, v'h) = v; build_0 = build_0.store_ref(v'a); build_0 = ~ null?(v'b) ? build_0.store_int(true, 1).store_ref(v'b) : build_0.store_int(false, 1); -build_0 = ~ null?(v'c) ? build_0.store_int(true, 1).store_ref(begin_cell().store_slice(v'c).end_cell()) : build_0.store_int(false, 1); var build_1 = begin_cell(); +build_1 = ~ null?(v'c) ? build_1.store_int(true, 1).store_ref(begin_cell().store_slice(v'c).end_cell()) : build_1.store_int(false, 1); build_1 = ~ null?(v'd) ? build_1.store_int(true, 1).store_ref(begin_cell().store_slice(v'd).end_cell()) : build_1.store_int(false, 1); build_1 = build_1.store_int(v'e, 1); build_1 = build_1.store_int(v'f, 257); @@ -13837,8 +13837,8 @@ if (null?(loaded)) { "code": { "code": "var v'a = sc_0~load_ref(); var v'b = sc_0~load_int(1) ? sc_0~load_ref() : null(); -var v'c = sc_0~load_int(1) ? sc_0~load_ref().begin_parse() : null(); slice sc_1 = sc_0~load_ref().begin_parse(); +var v'c = sc_1~load_int(1) ? sc_1~load_ref().begin_parse() : null(); var v'd = sc_1~load_int(1) ? sc_1~load_ref().begin_parse() : null(); var v'e = sc_1~load_int(1); var v'f = sc_1~load_int(257); diff --git a/src/storage/__snapshots__/resolveAllocation.spec.ts.snap b/src/storage/__snapshots__/resolveAllocation.spec.ts.snap index 8234d108c..c2a712c2f 100644 --- a/src/storage/__snapshots__/resolveAllocation.spec.ts.snap +++ b/src/storage/__snapshots__/resolveAllocation.spec.ts.snap @@ -465,12 +465,12 @@ exports[`resolveAllocation should write program 1`] = ` ], "size": { "bits": 514, - "refs": 0, + "refs": 1, }, }, "size": { "bits": 514, - "refs": 0, + "refs": 1, }, }, "type": { @@ -867,7 +867,7 @@ exports[`resolveAllocation should write program 1`] = ` ], "size": { "bits": 771, - "refs": 0, + "refs": 1, }, }, "ops": [ @@ -916,7 +916,7 @@ exports[`resolveAllocation should write program 1`] = ` ], "size": { "bits": 771, - "refs": 0, + "refs": 1, }, }, "ops": [ @@ -965,12 +965,12 @@ exports[`resolveAllocation should write program 1`] = ` ], "size": { "bits": 771, - "refs": 0, + "refs": 1, }, }, "size": { "bits": 771, - "refs": 0, + "refs": 1, }, }, "type": { @@ -1667,7 +1667,7 @@ exports[`resolveAllocation should write program 1`] = ` "ref": false, "size": { "bits": 771, - "refs": 0, + "refs": 1, }, "type": "Deep", }, @@ -1685,7 +1685,7 @@ exports[`resolveAllocation should write program 1`] = ` "ref": false, "size": { "bits": 771, - "refs": 0, + "refs": 1, }, "type": "Deep", }, @@ -1703,7 +1703,7 @@ exports[`resolveAllocation should write program 1`] = ` "ref": false, "size": { "bits": 771, - "refs": 0, + "refs": 1, }, "type": "Deep", }, @@ -1727,7 +1727,7 @@ exports[`resolveAllocation should write program 1`] = ` "ref": false, "size": { "bits": 771, - "refs": 0, + "refs": 1, }, "type": "Deep", }, @@ -1740,7 +1740,7 @@ exports[`resolveAllocation should write program 1`] = ` ], "size": { "bits": 771, - "refs": 0, + "refs": 1, }, }, "ops": [ @@ -1752,7 +1752,7 @@ exports[`resolveAllocation should write program 1`] = ` "ref": false, "size": { "bits": 771, - "refs": 0, + "refs": 1, }, "type": "Deep", }, @@ -1765,7 +1765,7 @@ exports[`resolveAllocation should write program 1`] = ` ], "size": { "bits": 771, - "refs": 0, + "refs": 2, }, }, "ops": [ @@ -1777,7 +1777,7 @@ exports[`resolveAllocation should write program 1`] = ` "ref": false, "size": { "bits": 771, - "refs": 0, + "refs": 1, }, "type": "Deep", }, @@ -1790,12 +1790,12 @@ exports[`resolveAllocation should write program 1`] = ` ], "size": { "bits": 771, - "refs": 0, + "refs": 2, }, }, "size": { "bits": 771, - "refs": 0, + "refs": 2, }, }, "type": { diff --git a/src/storage/allocator.ts b/src/storage/allocator.ts index 8aad1800b..35ab3c3b3 100644 --- a/src/storage/allocator.ts +++ b/src/storage/allocator.ts @@ -301,16 +301,27 @@ function allocateSegment( let next: AllocationCell | null = null; const used: { bits: number; refs: number } = { bits: 0, refs: 0 }; + const [totalBits, totalRefs] = ops.reduce( + ([sumBits, sumRefs], op) => { + const opSize = getOperationSize(op.op); + return [sumBits + opSize.bits, sumRefs + opSize.refs]; + }, + [0, 0], + ); + // check if there will be a new allocation segment, + // meaning we need to have a spare ref to link it + // from the current segment + if (totalBits > bits || totalRefs > refs) { + refs -= 1; + } + for (const [i, op] of ops.entries()) { const size = getOperationSize(op.op); // Check if we can fit this operation if (size.bits > bits || size.refs > refs) { - next = allocateSegment( - ops.slice(i), - 1023 - ALLOCATOR_RESERVE_BIT, - 4 - ALLOCATOR_RESERVE_REF, - ); + used.refs += 1; + next = allocateSegment(ops.slice(i), 1023, 4); break; } diff --git a/src/test/e2e-emulated/__snapshots__/intrinsics.spec.ts.snap b/src/test/e2e-emulated/__snapshots__/intrinsics.spec.ts.snap index 39469aed6..0c72cc34a 100644 --- a/src/test/e2e-emulated/__snapshots__/intrinsics.spec.ts.snap +++ b/src/test/e2e-emulated/__snapshots__/intrinsics.spec.ts.snap @@ -25,7 +25,7 @@ exports[`intrinsics should return correct intrinsic results 1`] = ` }, { "$type": "processed", - "gasUsed": 7881n, + "gasUsed": 7915n, }, { "$type": "sent", diff --git a/src/test/e2e-emulated/__snapshots__/map.spec.ts.snap b/src/test/e2e-emulated/__snapshots__/map.spec.ts.snap index a61876748..aac580bf2 100644 --- a/src/test/e2e-emulated/__snapshots__/map.spec.ts.snap +++ b/src/test/e2e-emulated/__snapshots__/map.spec.ts.snap @@ -18,7 +18,7 @@ exports[`map should implement maps correctly 1`] = ` }, "bounce": true, "from": "@treasure(treasure)", - "to": "kQDF6S3qUJdr9fVijosUCNJyi5YNnAe1KvoyKjWqsnky6crw", + "to": "kQA9A-B3CjPFrYe-tOHzjlnaFNzrG5eTlY2AmsgtGfkzSLTM", "type": "internal", "value": "1", }, @@ -36,10 +36,10 @@ exports[`map should implement maps correctly 1`] = ` "type": "cell", }, "bounce": false, - "from": "kQDF6S3qUJdr9fVijosUCNJyi5YNnAe1KvoyKjWqsnky6crw", + "from": "kQA9A-B3CjPFrYe-tOHzjlnaFNzrG5eTlY2AmsgtGfkzSLTM", "to": "@treasure(treasure)", "type": "internal", - "value": "0.981951", + "value": "0.982029", }, }, ], diff --git a/src/test/e2e-emulated/allocation.spec.ts b/src/test/e2e-emulated/allocation.spec.ts new file mode 100644 index 000000000..c6723cbce --- /dev/null +++ b/src/test/e2e-emulated/allocation.spec.ts @@ -0,0 +1,32 @@ +import { toNano } from "@ton/core"; +import { ContractSystem } from "@tact-lang/emulator"; +import { Test } from "./contracts/output/allocation_Test"; + +describe("allocation", () => { + it("should deploy correctly and process SetCost message without cell overflow", async () => { + const system = await ContractSystem.create(); + const owner = system.treasure("owner"); + const contract = system.open( + await Test.fromInit(owner.address, { + $$type: "Struct2", + c: "", + d: "", + e: "", + f: "", + }), + ); + system.name(contract.address, "main"); + await contract.send( + owner, + { value: toNano(1) }, + { $$type: "Deploy", queryId: 0n }, + ); + await system.run(); + await contract.send( + owner, + { value: toNano(1) }, + { $$type: "SetCost", cost: toNano("0.1") }, + ); + await system.run(); + }); +}); diff --git a/src/test/e2e-emulated/contracts/allocation.tact b/src/test/e2e-emulated/contracts/allocation.tact new file mode 100644 index 000000000..de04f65b7 --- /dev/null +++ b/src/test/e2e-emulated/contracts/allocation.tact @@ -0,0 +1,37 @@ +import "@stdlib/deploy"; + +message SetCost { + cost: Int? as coins; +} + +struct Struct2 { + c: String; + d: String; + e: String; + f: String; +} + +contract Test with Deployable { + owner: Address; + seqno: Int as uint256 = 0; + a: Int as uint256 = 0; + b: Int as uint256 = 0; + + struct2: Struct2; + + author3: Address?; + + cost: Int? as coins; + address: Address?; + price: Int? as coins; + + init(owner: Address, struct2: Struct2) { + self.owner = owner; + self.struct2 = struct2; + } + + receive(msg: SetCost) { + self.cost = msg.cost; + self.reply("Cost is updated".asComment()); + } +} \ No newline at end of file diff --git a/tact.config.json b/tact.config.json index 205786a5c..ef70b2028 100644 --- a/tact.config.json +++ b/tact.config.json @@ -370,6 +370,11 @@ "name": "getter-names-conflict", "path": "./src/test/e2e-emulated/contracts/getter-names-conflict.tact", "output": "./src/test/e2e-emulated/contracts/output" + }, + { + "name": "allocation", + "path": "./src/test/e2e-emulated/contracts/allocation.tact", + "output": "./src/test/e2e-emulated/contracts/output" } ] }