diff --git a/packages/cashscript/src/debugging.ts b/packages/cashscript/src/debugging.ts index ca1d17dcc..31a90b392 100644 --- a/packages/cashscript/src/debugging.ts +++ b/packages/cashscript/src/debugging.ts @@ -1,4 +1,4 @@ -import { AuthenticationErrorCommon, AuthenticationInstruction, AuthenticationProgramCommon, AuthenticationProgramStateCommon, WalletTemplate, WalletTemplateScriptUnlocking, binToHex, createCompiler, decodeAuthenticationInstructions, encodeAuthenticationInstruction, walletTemplateToCompilerConfiguration } from '@bitauth/libauth'; +import { AuthenticationErrorBch2025Additions, AuthenticationErrorCommon, AuthenticationInstruction, AuthenticationProgramCommon, AuthenticationProgramStateCommon, WalletTemplate, WalletTemplateScriptUnlocking, binToHex, createCompiler, decodeAuthenticationInstructions, encodeAuthenticationInstruction, walletTemplateToCompilerConfiguration } from '@bitauth/libauth'; import { Artifact, LogData, LogEntry, Op, PrimitiveType, StackItem, asmToBytecode, bytecodeToAsm, decodeBool, decodeInt, decodeString } from '@cashscript/utils'; import { findLastIndex, toRegExp } from './utils.js'; import { FailedRequireError, FailedTransactionError, FailedTransactionEvaluationError } from './Errors.js'; @@ -93,7 +93,10 @@ const debugSingleScenario = ( } } - const lastExecutedDebugStep = executedDebugSteps[executedDebugSteps.length - 1]; + // Instructions in a branch that is not taken are skipped, but still cost operations, so a resource limit can be reached on + // one of them. Evaluation stops at the step that raised an error, so that step is kept even if it was skipped. + const finalDebugStep = lockingScriptDebugResult[lockingScriptDebugResult.length - 1]; + const lastExecutedDebugStep = finalDebugStep?.error ? finalDebugStep : executedDebugSteps[executedDebugSteps.length - 1]; // If an error is present in the last step, that means a require statement in the middle of the function failed if (lastExecutedDebugStep.error) { @@ -122,7 +125,11 @@ const debugSingleScenario = ( } const frame = resolveFrame(artifact, lastExecutedDebugStep); - const requireStatement = frame.requires.find((statement) => statement.ip === requireStatementIp); + // A resource limit can run out on any instruction, including the one a require statement ends in, so it is never + // attributed to that require: its condition did not fail, the transaction ran out of budget + const requireStatement = isResourceLimitError(error) + ? undefined + : frame.requires.find((statement) => statement.ip === requireStatementIp); if (requireStatement) { const callStack = buildCallStack(artifact, lastExecutedDebugStep, frame, requireStatement, failingIp); @@ -395,3 +402,13 @@ const verifyFullTransaction = (template: WalletTemplate): void => { const isSignatureCheckWithoutVerify = (instruction: AuthenticationInstruction): boolean => { return [Op.OP_CHECKSIG, Op.OP_CHECKMULTISIG, Op.OP_CHECKDATASIG].includes(instruction.opcode); }; + +// Budgets that are counted per operation, so they can run out on the instruction a require statement ends in +const RESOURCE_LIMIT_ERRORS: string[] = [ + AuthenticationErrorBch2025Additions.excessiveOperationCost, + AuthenticationErrorBch2025Additions.excessiveHashing, + AuthenticationErrorCommon.exceededMaximumSignatureCheckCount, + AuthenticationErrorCommon.exceededMaximumOperationCount, +]; + +const isResourceLimitError = (error: string): boolean => RESOURCE_LIMIT_ERRORS.some((limit) => error.includes(limit)); diff --git a/packages/cashscript/test/debugging.test.ts b/packages/cashscript/test/debugging.test.ts index a999c8646..887233fb5 100644 --- a/packages/cashscript/test/debugging.test.ts +++ b/packages/cashscript/test/debugging.test.ts @@ -1,11 +1,12 @@ -import { Contract, MockNetworkProvider, SignatureAlgorithm, SignatureTemplate, TransactionBuilder, UnlockerLockingBytecodeMismatchError, VmTarget } from '../src/index.js'; +import { Contract, FailedTransactionEvaluationError, MockNetworkProvider, SignatureAlgorithm, SignatureTemplate, TransactionBuilder, UnlockerLockingBytecodeMismatchError, VmTarget } from '../src/index.js'; import { DEFAULT_VM_TARGET, getLockScriptName } from '../src/libauth-template/utils.js'; import { aliceAddress, alicePriv, alicePub, bobPriv, bobPub } from './fixture/vars.js'; import { randomUtxo } from '../src/utils.js'; -import { AuthenticationErrorCommon, binToHex, hexToBin } from '@bitauth/libauth'; +import { AuthenticationErrorBch2025Additions, AuthenticationErrorCommon, binToHex, hexToBin } from '@bitauth/libauth'; import { artifactTestMultipleConstructorParameters, artifactTestLogs, + artifactTestOperationCostOverrun, artifactTestConsecutiveLogs, artifactTestMultipleLogs, artifactTestRequires, @@ -638,6 +639,42 @@ describe('Debugging tests', () => { ].value`); expect(() => transaction.debug()).toThrow(`Reason: ${AuthenticationErrorCommon.invalidTransactionOutputIndex}`); }); + + // An operation cost overrun can stop evaluation on any instruction: the one a require statement ends in, or one in a + // skipped branch, which still costs operations + it('should report an operation cost overrun as such, not as a failed require statement', async () => { + const contract = new Contract(artifactTestOperationCostOverrun, [], { provider }); + const requireIps = artifactTestOperationCostOverrun.debug!.requires.map((statement) => statement.ip); + let overrunsOnRequires = 0; + + // The rounds spend most of the budget and the padding sets how large it is, so scanning both moves the point where + // it runs out across the instructions of the loop and of the require statements after it + for (let rounds = 37n; rounds <= 45n; rounds++) { + for (let padding = 0; ; padding++) { + const utxo = provider.addUtxo(contract.address, randomUtxo()); + const transaction = new TransactionBuilder({ provider }) + .addInput(utxo, contract.unlock.spend(rounds, new Uint8Array(padding))) + .addOutput({ to: contract.address, amount: 1000n }); + + let failure: unknown; + try { + transaction.debug(); + break; + } catch (error) { + failure = error; + } + + // The require conditions all hold, so every failure is the overrun + expect(failure).toBeInstanceOf(FailedTransactionEvaluationError); + const { libauthErrorMessage, failingInstructionPointer } = failure as FailedTransactionEvaluationError; + expect(libauthErrorMessage).toContain(AuthenticationErrorBch2025Additions.excessiveOperationCost); + if (requireIps.includes(failingInstructionPointer)) overrunsOnRequires++; + } + } + + // The scan has to hit the case #456 is about: the budget running out on a require statement's own instruction + expect(overrunsOnRequires).toBeGreaterThan(0); + }); }); describe('Template encoding', () => { diff --git a/packages/cashscript/test/fixture/debugging/debugging_contracts.ts b/packages/cashscript/test/fixture/debugging/debugging_contracts.ts index f957f817b..644d721e1 100644 --- a/packages/cashscript/test/fixture/debugging/debugging_contracts.ts +++ b/packages/cashscript/test/fixture/debugging/debugging_contracts.ts @@ -592,6 +592,21 @@ contract Test(pubkey owner, int num, int num2, int num3, int num4, int num5) { } `; +// The loop spends most of the operation cost budget, and the padding sets how much budget the input gets, so a test can +// move the point where it runs out across the instructions of the require statements after the loop +const CONTRACT_TEST_OPERATION_COST_OVERRUN = ` +contract Test() { + function spend(int rounds, bytes unused padding) { + bytes32 digest = sha256(0x00); + for (int i = 0; i < rounds; i = i + 1) { + digest = sha256(digest); + } + require(tx.outputs[0].lockingBytecode == tx.inputs[0].lockingBytecode, "Output 0 must pay back to the contract"); + require(tx.outputs[0].value >= 1000, "Output 0 must keep 1000 sats"); + } +} +`; + export const artifactTestRequires = compileString(CONTRACT_TEST_REQUIRES); export const artifactTestSingleFunction = compileString(CONTRACT_TEST_REQUIRE_SINGLE_FUNCTION); export const artifactTestMultilineRequires = compileString(CONTRACT_TEST_MULTILINE_REQUIRES); @@ -618,6 +633,7 @@ export const artifactTestNestedImportedFunctions = compileString( ); export const artifactTestMixedNestedFunctions = compileString(CONTRACT_TEST_MIXED_NESTED_FUNCTIONS); export const artifactTestInlinedCallingDefined = compileString(CONTRACT_TEST_INLINED_CALLING_DEFINED); +export const artifactTestOperationCostOverrun = compileString(CONTRACT_TEST_OPERATION_COST_OVERRUN); // Compiled from a file so the imported function (function_helpers.cash) keeps its own source provenance. export const artifactTestImportedFunctionDebugging = compileFile(new URL('./function_importer.cash', import.meta.url)); diff --git a/website/docs/releases/release-notes.md b/website/docs/releases/release-notes.md index cb7dd3c95..eeb8a7f37 100644 --- a/website/docs/releases/release-notes.md +++ b/website/docs/releases/release-notes.md @@ -38,6 +38,7 @@ This release contains several breaking changes, please refer to the [migration n - :bug: Fix bug where invalid hex strings were silently encoded to different bytes. - :bug: Fix bug where `pubkey` arguments were not checked to be 33 or 65 bytes. - :bug: Fix bug in `ElectrumNetworkProvider` automatic connection management. +- :bug: Fix bug where debugging reported a VM resource limit as a failed require statement. - :bug: Fix minimum fee check rounding bug. - :bug: Fix bug where `setLocktime()` accepted invalid values. - :bug: Fix edge case bug where a failing final `require(variable)` statement was reported incorrectly in debugging.