diff --git a/src/libxrpl/tx/ApplyContext.cpp b/src/libxrpl/tx/ApplyContext.cpp index 05c6425634b..4c839acef24 100644 --- a/src/libxrpl/tx/ApplyContext.cpp +++ b/src/libxrpl/tx/ApplyContext.cpp @@ -58,14 +58,19 @@ ApplyContext::discard() std::optional ApplyContext::apply(TER ter) { - if (vmReturnCode_.has_value()) + // tecINTERNAL reports an xrpld bug, not a result: nothing the VM recorded + // before we hit it belongs in the metadata. + if (ter != tecINTERNAL) { + if (vmReturnCode_.has_value()) + { + // NOLINTNEXTLINE(bugprone-unchecked-optional-access) view_ emplaced in constructor + view_->setVMReturnCode(*vmReturnCode_); + } // NOLINTNEXTLINE(bugprone-unchecked-optional-access) view_ emplaced in constructor - view_->setVMReturnCode(*vmReturnCode_); + view_->setGasUsed(gasUsed_); } // NOLINTNEXTLINE(bugprone-unchecked-optional-access) view_ emplaced in constructor - view_->setGasUsed(gasUsed_); - // NOLINTNEXTLINE(bugprone-unchecked-optional-access) view_ emplaced in constructor return view_->apply(base_, tx, ter, parentBatchId_, (flags_ & TapDryRun) != 0u, journal); } diff --git a/src/libxrpl/tx/transactors/escrow/EscrowFinish.cpp b/src/libxrpl/tx/transactors/escrow/EscrowFinish.cpp index a44b77c006d..1d09d55b89c 100644 --- a/src/libxrpl/tx/transactors/escrow/EscrowFinish.cpp +++ b/src/libxrpl/tx/transactors/escrow/EscrowFinish.cpp @@ -35,7 +35,6 @@ #include #include -#include #include #include #include @@ -417,42 +416,49 @@ EscrowFinish::doApply() return tecINTERNAL; } std::uint32_t const allowance = ctx_.tx[sfGas]; - auto re = runEscrowWasm(wasm, ledgerDataProvider, allowance, escrowFunctionName); + auto const re = runEscrowWasm(wasm, ledgerDataProvider, allowance, escrowFunctionName); JLOG(j_.trace()) << "Escrow WASM ran"; - if (auto const& data = ledgerDataProvider.getData(); data.has_value()) + // Gas consumed, reported in the tx metadata whenever the engine has a + // trustworthy number: a completed run, out of gas, or a wasm fault. + std::optional const cost = re.has_value() ? re->cost : re.error().cost; + if (cost.has_value()) { - if (data->size() > kMaxWasmDataLength) - { - // should already be checked in the updateData host function + // The engine cannot spend more than it was given, and pins the cost + // to the allowance when it runs out. + if (*cost < 0 || *cost > allowance) return tecINTERNAL; // LCOV_EXCL_LINE - } - slep->setFieldVL(sfData, makeSlice(*data)); - ctx_.view().update(slep); + ctx_.setGasUsed(static_cast(*cost)); } - if (re.has_value()) + if (!re.has_value()) { - auto const reValue = re.value().result; - auto const reCost = re.value().cost; - JLOG(j_.debug()) << "WASM Success: " + std::to_string(reValue) << ", cost: " << reCost; + // No return code, and any data it wrote goes away with the view. + JLOG(j_.debug()) << "WASM Failure: " + transHuman(re.error().ter); + return re.error().ter; + } - ctx_.setVMReturnCode(reValue); + auto const reValue = re->result; + JLOG(j_.debug()) << "WASM Success: " + std::to_string(reValue) << ", cost: " << re->cost; - if (reCost < 0 || reCost > std::numeric_limits::max()) - return tecINTERNAL; // LCOV_EXCL_LINE - ctx_.setGasUsed(static_cast(reCost)); + ctx_.setVMReturnCode(reValue); - if (reValue <= 0) + // Only matters on a reject, where the escrow survives: + // Transactor::processPersistentChanges replays this after the reset. + if (auto const& data = ledgerDataProvider.getData(); data.has_value()) + { + if (data->size() > kMaxWasmDataLength) { - return tecBYTECODE_REJECTED; + // should already be checked in the updateData host function + return tecINTERNAL; // LCOV_EXCL_LINE } + slep->setFieldVL(sfData, makeSlice(*data)); + ctx_.view().update(slep); } - else - { - JLOG(j_.debug()) << "WASM Failure: " + transHuman(re.error().ter); - return re.error().ter; - } + + // 0 or negative is a contract-defined reject code, reported as sfVMReturnCode. + if (reValue <= 0) + return tecBYTECODE_REJECTED; } AccountID const account = (*slep)[sfAccount]; diff --git a/src/test/app/EscrowSmart_test.cpp b/src/test/app/EscrowSmart_test.cpp index e85cbb30b39..f8c2c3f2cf1 100644 --- a/src/test/app/EscrowSmart_test.cpp +++ b/src/test/app/EscrowSmart_test.cpp @@ -492,6 +492,18 @@ struct EscrowSmart_test : public beast::unit_test::Suite Fee(finishFee), escrow::Gas(2), Ter(tecOUT_OF_GAS)); + + // Running out of gas still reports the gas consumed, which is the + // whole allowance. The function did not run to completion, so + // there is no return code to report. + auto const txMeta = env.meta(); + if (BEAST_EXPECT(txMeta && txMeta->isFieldPresent(sfGasUsed))) + { + BEAST_EXPECTS( + txMeta->getFieldU32(sfGasUsed) == 2, + std::to_string(txMeta->getFieldU32(sfGasUsed))); + } + BEAST_EXPECT(txMeta && !txMeta->isFieldPresent(sfVMReturnCode)); } { @@ -510,6 +522,32 @@ struct EscrowSmart_test : public beast::unit_test::Suite escrow::Gas(allowance), Ter(tefNO_BYTECODE)); } + + { + // a trap in the wasm code reports the gas it burned, which is only + // part of the allowance + auto const trapSeq = env.seq(alice); + env(escrow::create(alice, carol, XRP(500)), + escrow::Bytecode(kTrapUnreachableHex), + escrow::kCancelTime(env.now() + 100s), + Fee(env.current()->fees().base * 10 + kTrapUnreachableHex.size() / 2 * 5)); + env.close(); + + std::uint32_t const allowance = 1000; + env(escrow::finish(carol, alice, trapSeq), + Fee(env.current()->fees().base + + (allowance * env.current()->fees().gasPrice) / microDropsPerDrop + 1), + escrow::Gas(allowance), + Ter(tecFAILED_PROCESSING)); + + auto const txMeta = env.meta(); + if (BEAST_EXPECT(txMeta && txMeta->isFieldPresent(sfGasUsed))) + { + auto const gasUsed = txMeta->getFieldU32(sfGasUsed); + BEAST_EXPECTS(gasUsed < allowance, std::to_string(gasUsed)); + } + BEAST_EXPECT(txMeta && !txMeta->isFieldPresent(sfVMReturnCode)); + } } void