diff --git a/src/test/app/Escrow_test.cpp b/src/test/app/Escrow_test.cpp index 6721eed12f..2923ea84ca 100644 --- a/src/test/app/Escrow_test.cpp +++ b/src/test/app/Escrow_test.cpp @@ -1756,7 +1756,8 @@ struct Escrow_test : public beast::unit_test::suite return cfg; }), features); - XRPAmount const txnFees = env.current()->fees().base * 10 + 1000; + XRPAmount const txnFees = + env.current()->fees().base * 10 + wasmHex.size() / 2 * 5; // create escrow env.fund(XRP(5000), alice, carol); @@ -1767,7 +1768,7 @@ struct Escrow_test : public beast::unit_test::suite // FinishFunction + CancelAfter env(escrowCreate, escrow::finish_function(wasmHex), - escrow::cancel_time(env.now() + 100s), + escrow::cancel_time(env.now() + 20s), fee(txnFees)); env.close(); } @@ -1775,7 +1776,7 @@ struct Escrow_test : public beast::unit_test::suite // FinishFunction + Condition + CancelAfter env(escrowCreate, escrow::finish_function(wasmHex), - escrow::cancel_time(env.now() + 100s), + escrow::cancel_time(env.now() + 30s), escrow::condition(escrow::cb1), fee(txnFees)); env.close(); @@ -1784,7 +1785,7 @@ struct Escrow_test : public beast::unit_test::suite // FinishFunction + FinishAfter + CancelAfter env(escrowCreate, escrow::finish_function(wasmHex), - escrow::cancel_time(env.now() + 100s), + escrow::cancel_time(env.now() + 40s), escrow::finish_time(env.now() + 2s), fee(txnFees)); env.close(); @@ -1793,7 +1794,7 @@ struct Escrow_test : public beast::unit_test::suite // FinishFunction + FinishAfter + Condition + CancelAfter env(escrowCreate, escrow::finish_function(wasmHex), - escrow::cancel_time(env.now() + 100s), + escrow::cancel_time(env.now() + 50s), escrow::condition(escrow::cb1), escrow::finish_time(env.now() + 2s), fee(txnFees)); @@ -1841,11 +1842,20 @@ struct Escrow_test : public beast::unit_test::suite // FinishFunction 0 length env(escrowCreate, escrow::finish_function(""), - escrow::cancel_time(env.now() + 100s), + escrow::cancel_time(env.now() + 60s), fee(txnFees), ter(temMALFORMED)); env.close(); } + { + // Not enough fees + env(escrowCreate, + escrow::finish_function(wasmHex), + escrow::cancel_time(env.now() + 70s), + fee(txnFees - 1), + ter(telINSUF_FEE_P)); + env.close(); + } { // FinishFunction nonexistent host function @@ -1890,7 +1900,8 @@ struct Escrow_test : public beast::unit_test::suite // featureSmartEscrow disabled Env env(*this, features - featureSmartEscrow); env.fund(XRP(5000), alice, carol); - XRPAmount const txnFees = env.current()->fees().base + 1000; + XRPAmount const txnFees = + env.current()->fees().base * 10 + wasmHex.size() / 2 * 5; env(escrow::finish(carol, alice, 1), fee(txnFees), escrow::comp_allowance(4), @@ -1929,7 +1940,8 @@ struct Escrow_test : public beast::unit_test::suite for (auto i = env.current()->seq(); i <= 257; ++i) env.close(); - XRPAmount const txnFees = env.current()->fees().base + 1000; + XRPAmount const txnFees = + env.current()->fees().base * 10 + wasmHex.size() / 2 * 5; env.fund(XRP(5000), alice, carol); // create escrow @@ -1986,7 +1998,10 @@ struct Escrow_test : public beast::unit_test::suite auto const allowance = 100; env(escrow::finish(carol, alice, seq2), - fee(env.current()->fees().base + allowance), + fee(env.current()->fees().base + + (allowance * env.current()->fees().gasPrice) / + MICRO_DROPS_PER_DROP + + 1), escrow::comp_allowance(allowance), ter(tefNO_WASM)); } @@ -2007,6 +2022,17 @@ struct Escrow_test : public beast::unit_test::suite // getLedgerSqn() >= 5} auto const& wasmHex = ledgerSqnWasmHex; std::uint32_t const allowance = 66; + auto escrowCreate = escrow::create(alice, carol, XRP(1000)); + auto [createFee, finishFee] = [&]() { + Env env(*this, features); + auto createFee = + env.current()->fees().base * 10 + wasmHex.size() / 2 * 5; + auto finishFee = env.current()->fees().base + + (allowance * env.current()->fees().gasPrice) / + MICRO_DROPS_PER_DROP + + 1; + return std::make_pair(createFee, finishFee); + }(); { // basic FinishFunction situation @@ -2015,38 +2041,36 @@ struct Escrow_test : public beast::unit_test::suite env.fund(XRP(5000), alice, carol); auto const seq = env.seq(alice); BEAST_EXPECT((*env.le(alice))[sfOwnerCount] == 0); - auto escrowCreate = escrow::create(alice, carol, XRP(1000)); - XRPAmount txnFees = env.current()->fees().base + 1000; env(escrowCreate, escrow::finish_function(wasmHex), escrow::cancel_time(env.now() + 100s), - fee(txnFees)); + fee(createFee)); env.close(); if (BEAST_EXPECT((*env.le(alice))[sfOwnerCount] == 2)) { - env.require(balance(alice, XRP(4000) - txnFees)); + env.require(balance(alice, XRP(4000) - createFee)); env.require(balance(carol, XRP(5000))); env(escrow::finish(carol, alice, seq), escrow::comp_allowance(allowance), - fee(txnFees), + fee(finishFee), ter(tecWASM_REJECTED)); env(escrow::finish(alice, alice, seq), escrow::comp_allowance(allowance), - fee(txnFees), + fee(finishFee), ter(tecWASM_REJECTED)); env(escrow::finish(alice, alice, seq), escrow::comp_allowance(allowance), - fee(txnFees), + fee(finishFee), ter(tecWASM_REJECTED)); env(escrow::finish(carol, alice, seq), escrow::comp_allowance(allowance), - fee(txnFees), + fee(finishFee), ter(tecWASM_REJECTED)); env(escrow::finish(carol, alice, seq), escrow::comp_allowance(allowance), - fee(txnFees), + fee(finishFee), ter(tecWASM_REJECTED)); env.close(); @@ -2059,7 +2083,7 @@ struct Escrow_test : public beast::unit_test::suite } env(escrow::finish(alice, alice, seq), - fee(txnFees), + fee(finishFee), escrow::comp_allowance(allowance), ter(tesSUCCESS)); @@ -2084,34 +2108,31 @@ struct Escrow_test : public beast::unit_test::suite BEAST_EXPECT((*env.le(alice))[sfOwnerCount] == 0); auto const seq = env.seq(alice); // create escrow - auto escrowCreate = escrow::create(alice, carol, XRP(1000)); - XRPAmount const createFee = env.current()->fees().base + 1000; env(escrowCreate, escrow::finish_function(wasmHex), escrow::condition(escrow::cb1), escrow::cancel_time(env.now() + 100s), fee(createFee)); env.close(); + auto const conditionFinishFee = finishFee + + env.current()->fees().base * (32 + (escrow::fb1.size() / 16)); if (BEAST_EXPECT((*env.le(alice))[sfOwnerCount] == 2)) { env.require(balance(alice, XRP(4000) - createFee)); env.require(balance(carol, XRP(5000))); - XRPAmount const txnFees = - env.current()->fees().base * 34 + 1000; - // no fulfillment provided, function fails env(escrow::finish(carol, alice, seq), escrow::comp_allowance(allowance), - fee(txnFees), + fee(finishFee), ter(tecCRYPTOCONDITION_ERROR)); // fulfillment provided, function fails env(escrow::finish(carol, alice, seq), escrow::condition(escrow::cb1), escrow::fulfillment(escrow::fb1), escrow::comp_allowance(allowance), - fee(txnFees), + fee(conditionFinishFee), ter(tecWASM_REJECTED)); if (BEAST_EXPECT(env.meta()->isFieldPresent(sfGasUsed))) BEAST_EXPECTS( @@ -2121,21 +2142,21 @@ struct Escrow_test : public beast::unit_test::suite // no fulfillment provided, function succeeds env(escrow::finish(alice, alice, seq), escrow::comp_allowance(allowance), - fee(txnFees), + fee(conditionFinishFee), ter(tecCRYPTOCONDITION_ERROR)); // wrong fulfillment provided, function succeeds env(escrow::finish(alice, alice, seq), escrow::condition(escrow::cb1), escrow::fulfillment(escrow::fb2), escrow::comp_allowance(allowance), - fee(txnFees), + fee(conditionFinishFee), ter(tecCRYPTOCONDITION_ERROR)); // fulfillment provided, function succeeds, tx succeeds env(escrow::finish(alice, alice, seq), escrow::condition(escrow::cb1), escrow::fulfillment(escrow::fb1), escrow::comp_allowance(allowance), - fee(txnFees), + fee(conditionFinishFee), ter(tesSUCCESS)); auto const txMeta = env.meta(); @@ -2158,37 +2179,35 @@ struct Escrow_test : public beast::unit_test::suite env.fund(XRP(5000), alice, carol); auto const seq = env.seq(alice); BEAST_EXPECT((*env.le(alice))[sfOwnerCount] == 0); - auto escrowCreate = escrow::create(alice, carol, XRP(1000)); - XRPAmount txnFees = env.current()->fees().base + 1000; auto const ts = env.now() + 97s; env(escrowCreate, escrow::finish_function(wasmHex), escrow::finish_time(ts), escrow::cancel_time(env.now() + 1000s), - fee(txnFees)); + fee(createFee)); env.close(); if (BEAST_EXPECT((*env.le(alice))[sfOwnerCount] == 2)) { - env.require(balance(alice, XRP(4000) - txnFees)); + env.require(balance(alice, XRP(4000) - createFee)); env.require(balance(carol, XRP(5000))); // finish time hasn't passed, function fails env(escrow::finish(carol, alice, seq), escrow::comp_allowance(allowance), - fee(txnFees + 1), + fee(finishFee + 1), ter(tecNO_PERMISSION)); env.close(); // finish time hasn't passed, function succeeds for (; env.now() < ts; env.close()) env(escrow::finish(carol, alice, seq), escrow::comp_allowance(allowance), - fee(txnFees + 2), + fee(finishFee + 2), ter(tecNO_PERMISSION)); env(escrow::finish(carol, alice, seq), escrow::comp_allowance(allowance), - fee(txnFees + 1), + fee(finishFee + 1), ter(tesSUCCESS)); auto const txMeta = env.meta(); @@ -2210,31 +2229,29 @@ struct Escrow_test : public beast::unit_test::suite env.fund(XRP(5000), alice, carol); auto const seq = env.seq(alice); BEAST_EXPECT((*env.le(alice))[sfOwnerCount] == 0); - auto escrowCreate = escrow::create(alice, carol, XRP(1000)); - XRPAmount txnFees = env.current()->fees().base + 1000; env(escrowCreate, escrow::finish_function(wasmHex), escrow::finish_time(env.now() + 2s), escrow::cancel_time(env.now() + 100s), - fee(txnFees)); + fee(createFee)); // Don't close the ledger here if (BEAST_EXPECT((*env.le(alice))[sfOwnerCount] == 2)) { - env.require(balance(alice, XRP(4000) - txnFees)); + env.require(balance(alice, XRP(4000) - createFee)); env.require(balance(carol, XRP(5000))); // finish time hasn't passed, function fails env(escrow::finish(carol, alice, seq), escrow::comp_allowance(allowance), - fee(txnFees), + fee(finishFee), ter(tecNO_PERMISSION)); env.close(); // finish time has passed, function fails env(escrow::finish(carol, alice, seq), escrow::comp_allowance(allowance), - fee(txnFees), + fee(finishFee), ter(tecWASM_REJECTED)); if (BEAST_EXPECT(env.meta()->isFieldPresent(sfGasUsed))) BEAST_EXPECTS( @@ -2244,7 +2261,7 @@ struct Escrow_test : public beast::unit_test::suite // finish time has passed, function succeeds, tx succeeds env(escrow::finish(carol, alice, seq), escrow::comp_allowance(allowance), - fee(txnFees), + fee(finishFee), ter(tesSUCCESS)); auto const txMeta = env.meta(); @@ -2282,7 +2299,8 @@ struct Escrow_test : public beast::unit_test::suite auto const seq = env.seq(alice); BEAST_EXPECT((*env.le(alice))[sfOwnerCount] == 0); auto escrowCreate = escrow::create(alice, carol, XRP(1000)); - XRPAmount txnFees = env.current()->fees().base + 1000; + XRPAmount txnFees = + env.current()->fees().base * 10 + wasmHex.size() / 2 * 5; env(escrowCreate, escrow::finish_function(wasmHex), escrow::finish_time(env.now() + 11s), @@ -2291,17 +2309,23 @@ struct Escrow_test : public beast::unit_test::suite fee(txnFees)); env.close(); - if (BEAST_EXPECT((*env.le(alice))[sfOwnerCount] == 2)) + if (BEAST_EXPECT( + (*env.le(alice))[sfOwnerCount] == + (1 + wasmHex.size() / 2 / 500))) { env.require(balance(alice, XRP(4000) - txnFees)); env.require(balance(carol, XRP(5000))); auto const allowance = 1'000'000; + XRPAmount const finishFee = env.current()->fees().base + + (allowance * env.current()->fees().gasPrice) / + MICRO_DROPS_PER_DROP + + 1; // FinishAfter time hasn't passed env(escrow::finish(carol, alice, seq), escrow::comp_allowance(allowance), - fee(txnFees), + fee(finishFee), ter(tecNO_PERMISSION)); env.close(); env.close(); @@ -2314,7 +2338,7 @@ struct Escrow_test : public beast::unit_test::suite env(escrow::finish(alice, alice, seq), escrow::comp_allowance(allowance), - fee(txnFees), + fee(finishFee), ter(tesSUCCESS)); auto const txMeta = env.meta(); @@ -2392,7 +2416,8 @@ struct Escrow_test : public beast::unit_test::suite env.seq(alice) == 20, std::to_string(env.seq(alice)))) { auto const seq = env.seq(alice); - XRPAmount txnFees = env.current()->fees().base + 1000; + XRPAmount txnFees = + env.current()->fees().base * 10 + wasmHex.size() / 2 * 5; env(escrow::create(alice, carol, XRP(1000)), escrow::finish_function(wasmHex), escrow::finish_time(env.now() + 2s), @@ -2403,10 +2428,13 @@ struct Escrow_test : public beast::unit_test::suite env.close(); auto const allowance = 137'596; - + auto const finishFee = env.current()->fees().base + + (allowance * env.current()->fees().gasPrice) / + MICRO_DROPS_PER_DROP + + 1; env(escrow::finish(carol, alice, seq), escrow::comp_allowance(allowance), - fee(txnFees)); + fee(finishFee)); env.close(); auto const txMeta = env.meta(); diff --git a/src/xrpld/app/tx/detail/Escrow.cpp b/src/xrpld/app/tx/detail/Escrow.cpp index 4b5a92e8c2..d0b42e1615 100644 --- a/src/xrpld/app/tx/detail/Escrow.cpp +++ b/src/xrpld/app/tx/detail/Escrow.cpp @@ -127,8 +127,9 @@ EscrowCreate::calculateBaseFee(ReadView const& view, STTx const& tx) XRPAmount txnFees{Transactor::calculateBaseFee(view, tx)}; if (tx.isFieldPresent(sfFinishFunction)) { - // TODO: make this fee increase based on the extra compute run - txnFees += 1000; + // 10 base fees for the transaction (1 is in + // `Transactor::calculateBaseFee`), plus 5 drops per byte + txnFees += 9 * view.fees().base + 5 * tx[sfFinishFunction].size(); } return txnFees; } @@ -503,6 +504,17 @@ escrowLockApplyHelper( return tesSUCCESS; } +template +static uint32_t +calculateAdditionalReserve(T const& finishFunction) +{ + if (!finishFunction) + return 1; + // First 500 bytes included in the normal reserve + // Each additional 500 bytes requires an additional reserve + return 1 + (finishFunction->size() / 500); +} + TER EscrowCreate::doApply() { @@ -546,9 +558,11 @@ EscrowCreate::doApply() // Check reserve and funds availability STAmount const amount{ctx_.tx[sfAmount]}; + auto const reserveToAdd = + calculateAdditionalReserve(ctx_.tx[~sfFinishFunction]); auto const reserve = - ctx_.view().fees().accountReserve((*sle)[sfOwnerCount] + 1); + ctx_.view().fees().accountReserve((*sle)[sfOwnerCount] + reserveToAdd); if (mSourceBalance < reserve) return tecINSUFFICIENT_RESERVE; @@ -654,11 +668,7 @@ EscrowCreate::doApply() // increment owner count // TODO: determine actual reserve based on FinishFunction size - adjustOwnerCount( - ctx_.view(), - sle, - ctx_.tx.isFieldPresent(sfFinishFunction) ? 2 : 1, - ctx_.journal); + adjustOwnerCount(ctx_.view(), sle, reserveToAdd, ctx_.journal); ctx_.view().update(sle); return tesSUCCESS; } @@ -773,7 +783,11 @@ EscrowFinish::calculateBaseFee(ReadView const& view, STTx const& tx) } if (auto const allowance = tx[~sfComputationAllowance]; allowance) { - extraFee += (*allowance) * view.fees().gasPrice / MICRO_DROPS_PER_DROP; + // The extra fee is the allowance in drops, rounded up to the nearest + // whole drop. + // Integer math rounds down by default, so we add 1 to round up. + extraFee += + ((*allowance) * view.fees().gasPrice) / MICRO_DROPS_PER_DROP + 1; } return Transactor::calculateBaseFee(view, tx) + extraFee; } @@ -1375,13 +1389,12 @@ EscrowFinish::doApply() ctx_.view().update(sled); + auto const reserveToSubtract = + calculateAdditionalReserve((*slep)[~sfFinishFunction]); + // Adjust source owner count auto const sle = ctx_.view().peek(keylet::account(account)); - adjustOwnerCount( - ctx_.view(), - sle, - slep->isFieldPresent(sfFinishFunction) ? -2 : -1, - ctx_.journal); + adjustOwnerCount(ctx_.view(), sle, -1 * reserveToSubtract, ctx_.journal); ctx_.view().update(sle); // Remove escrow from ledger @@ -1592,11 +1605,9 @@ EscrowCancel::doApply() } } - adjustOwnerCount( - ctx_.view(), - sle, - slep->isFieldPresent(sfFinishFunction) ? -2 : -1, - ctx_.journal); + auto const reserveToSubtract = + calculateAdditionalReserve((*slep)[~sfFinishFunction]); + adjustOwnerCount(ctx_.view(), sle, -1 * reserveToSubtract, ctx_.journal); ctx_.view().update(sle); // Remove escrow from ledger