fix: ensure GasUsed shows up in the metadata even on tecWASM_REJECTED (#5633)

* always set gas used

* fix

* add tests

* clean up

src/test/app/Escrow_test.cpp

@@ -296,7 +296,7 @@ struct Escrow_test : public beast::unit_test::suite
         {
             testcase("Implied Finish Time (without fix1571)");
 
-            Env env(*this, testable_amendments() - fix1571);
+            Env env(*this, features - fix1571);
             auto const baseFee = env.current()->fees().base;
             env.fund(XRP(5000), "alice", "bob", "carol");
             env.close();
@@ -1696,7 +1696,7 @@ struct Escrow_test : public beast::unit_test::suite
     }
 
     void
-    testCreateFinishFunctionPreflight()
+    testCreateFinishFunctionPreflight(FeatureBitset features)
     {
         testcase("Test preflight checks involving FinishFunction");
 
@@ -1712,7 +1712,7 @@ struct Escrow_test : public beast::unit_test::suite
 
         {
             // featureSmartEscrow disabled
-            Env env(*this, testable_amendments() - featureSmartEscrow);
+            Env env(*this, features - featureSmartEscrow);
             env.fund(XRP(5000), alice, carol);
             XRPAmount const txnFees = env.current()->fees().base + 1000;
             auto escrowCreate = escrow::create(alice, carol, XRP(1000));
@@ -1726,10 +1726,13 @@ struct Escrow_test : public beast::unit_test::suite
 
         {
             // FinishFunction > max length
-            Env env(*this, envconfig([](std::unique_ptr<Config> cfg) {
-                cfg->FEES.extension_size_limit = 10;  // 10 bytes
-                return cfg;
-            }));
+            Env env(
+                *this,
+                envconfig([](std::unique_ptr<Config> cfg) {
+                    cfg->FEES.extension_size_limit = 10;  // 10 bytes
+                    return cfg;
+                }),
+                features);
             XRPAmount const txnFees = env.current()->fees().base + 1000;
             // create escrow
             env.fund(XRP(5000), alice, carol);
@@ -1746,10 +1749,13 @@ struct Escrow_test : public beast::unit_test::suite
             env.close();
         }
 
-        Env env(*this, envconfig([](std::unique_ptr<Config> cfg) {
-            cfg->START_UP = Config::FRESH;
-            return cfg;
-        }));
+        Env env(
+            *this,
+            envconfig([](std::unique_ptr<Config> cfg) {
+                cfg->START_UP = Config::FRESH;
+                return cfg;
+            }),
+            features);
         XRPAmount const txnFees = env.current()->fees().base * 10 + 1000;
         // create escrow
         env.fund(XRP(5000), alice, carol);
@@ -1866,7 +1872,7 @@ struct Escrow_test : public beast::unit_test::suite
     }
 
     void
-    testFinishWasmFailures()
+    testFinishWasmFailures(FeatureBitset features)
     {
         testcase("EscrowFinish Smart Escrow failures");
 
@@ -1882,7 +1888,7 @@ struct Escrow_test : public beast::unit_test::suite
 
         {
             // featureSmartEscrow disabled
-            Env env(*this, testable_amendments() - featureSmartEscrow);
+            Env env(*this, features - featureSmartEscrow);
             env.fund(XRP(5000), alice, carol);
             XRPAmount const txnFees = env.current()->fees().base + 1000;
             env(escrow::finish(carol, alice, 1),
@@ -1894,10 +1900,13 @@ struct Escrow_test : public beast::unit_test::suite
 
         {
             // ComputationAllowance > max compute limit
-            Env env(*this, envconfig([](std::unique_ptr<Config> cfg) {
-                cfg->FEES.extension_compute_limit = 1'000;  // in gas
-                return cfg;
-            }));
+            Env env(
+                *this,
+                envconfig([](std::unique_ptr<Config> cfg) {
+                    cfg->FEES.extension_compute_limit = 1'000;  // in gas
+                    return cfg;
+                }),
+                features);
             env.fund(XRP(5000), alice, carol);
             // Run past the flag ledger so that a Fee change vote occurs and
             // updates FeeSettings. (It also activates all supported
@@ -1913,8 +1922,8 @@ struct Escrow_test : public beast::unit_test::suite
         }
 
         {
-            uint32_t const allowance = 10'000;
-            Env env(*this);
+            uint32_t const allowance = 5'664;
+            Env env(*this, features);
             env.fund(XRP(5000), alice, carol);
             auto const seq = env.seq(alice);
             XRPAmount const createFee = env.current()->fees().base + 1000;
@@ -1937,9 +1946,13 @@ struct Escrow_test : public beast::unit_test::suite
                 escrow::comp_allowance(allowance),
                 fee(txnFees),
                 ter(tecWASM_REJECTED));
+
+            auto const txMeta = env.meta();
+            if (BEAST_EXPECT(txMeta->isFieldPresent(sfGasUsed)))
+                BEAST_EXPECT(txMeta->getFieldU32(sfGasUsed) == allowance);
         }
 
-        Env env(*this);
+        Env env(*this, features);
 
         // Run past the flag ledger so that a Fee change vote occurs and
         // updates FeeSettings. (It also activates all supported
@@ -2011,7 +2024,7 @@ struct Escrow_test : public beast::unit_test::suite
     }
 
     void
-    testFinishFunction()
+    testFinishFunction(FeatureBitset features)
     {
         testcase("Example escrow function");
 
@@ -2028,7 +2041,7 @@ struct Escrow_test : public beast::unit_test::suite
 
         {
             // basic FinishFunction situation
-            Env env(*this);
+            Env env(*this, features);
             // create escrow
             env.fund(XRP(5000), alice, carol);
             auto const seq = env.seq(alice);
@@ -2067,6 +2080,14 @@ struct Escrow_test : public beast::unit_test::suite
                     fee(txnFees),
                     ter(tecWASM_REJECTED));
                 env.close();
+
+                {
+                    auto const txMeta = env.meta();
+                    if (BEAST_EXPECT(txMeta->isFieldPresent(sfGasUsed)))
+                        BEAST_EXPECT(
+                            txMeta->getFieldU32(sfGasUsed) == allowance);
+                }
+
                 env(escrow::finish(alice, alice, seq),
                     fee(txnFees),
                     escrow::comp_allowance(allowance),
@@ -2082,7 +2103,7 @@ struct Escrow_test : public beast::unit_test::suite
 
         {
             // FinishFunction + Condition
-            Env env(*this);
+            Env env(*this, features);
             env.fund(XRP(5000), alice, carol);
             BEAST_EXPECT((*env.le(alice))[sfOwnerCount] == 0);
             auto const seq = env.seq(alice);
@@ -2116,6 +2137,9 @@ struct Escrow_test : public beast::unit_test::suite
                     escrow::comp_allowance(allowance),
                     fee(txnFees),
                     ter(tecWASM_REJECTED));
+                if (BEAST_EXPECT(env.meta()->isFieldPresent(sfGasUsed)))
+                    BEAST_EXPECT(
+                        env.meta()->getFieldU32(sfGasUsed) == allowance);
                 env.close();
                 // no fulfillment provided, function succeeds
                 env(escrow::finish(alice, alice, seq),
@@ -2148,7 +2172,7 @@ struct Escrow_test : public beast::unit_test::suite
 
         {
             // FinishFunction + FinishAfter
-            Env env(*this);
+            Env env(*this, features);
             // create escrow
             env.fund(XRP(5000), alice, carol);
             auto const seq = env.seq(alice);
@@ -2196,7 +2220,7 @@ struct Escrow_test : public beast::unit_test::suite
 
         {
             // FinishFunction + FinishAfter #2
-            Env env(*this);
+            Env env(*this, features);
             // create escrow
             env.fund(XRP(5000), alice, carol);
             auto const seq = env.seq(alice);
@@ -2227,6 +2251,9 @@ struct Escrow_test : public beast::unit_test::suite
                     escrow::comp_allowance(allowance),
                     fee(txnFees),
                     ter(tecWASM_REJECTED));
+                if (BEAST_EXPECT(env.meta()->isFieldPresent(sfGasUsed)))
+                    BEAST_EXPECT(
+                        env.meta()->getFieldU32(sfGasUsed) == allowance);
                 env.close();
                 // finish time has passed, function succeeds, tx succeeds
                 env(escrow::finish(carol, alice, seq),
@@ -2245,7 +2272,7 @@ struct Escrow_test : public beast::unit_test::suite
     }
 
     void
-    testAllHostFunctions()
+    testAllHostFunctions(FeatureBitset features)
     {
         testcase("Test all host functions");
 
@@ -2259,7 +2286,7 @@ struct Escrow_test : public beast::unit_test::suite
         Account const carol{"carol"};
 
         {
-            Env env(*this);
+            Env env(*this, features);
             // create escrow
             env.fund(XRP(5000), alice, carol);
             auto const seq = env.seq(alice);
@@ -2311,7 +2338,7 @@ struct Escrow_test : public beast::unit_test::suite
     }
 
     void
-    testKeyletHostFunctions()
+    testKeyletHostFunctions(FeatureBitset features)
     {
         testcase("Test all keylet host functions");
 
@@ -2371,12 +2398,17 @@ struct Escrow_test : public beast::unit_test::suite
                 env.close();
                 env.close();
 
-                auto const allowance = 1'000'000;
+                auto const allowance = 99'596;
 
                 env(escrow::finish(carol, alice, seq),
                     escrow::comp_allowance(allowance),
                     fee(txnFees));
                 env.close();
+
+                auto const txMeta = env.meta();
+                if (BEAST_EXPECT(txMeta && txMeta->isFieldPresent(sfGasUsed)))
+                    BEAST_EXPECT(txMeta->getFieldU32(sfGasUsed) == allowance);
+                BEAST_EXPECT((*env.le(alice))[sfOwnerCount] == 12);
             }
         }
     }
@@ -2396,13 +2428,13 @@ struct Escrow_test : public beast::unit_test::suite
         testConsequences(features);
         testEscrowWithTickets(features);
         testCredentials(features);
-        testCreateFinishFunctionPreflight();
-        testFinishWasmFailures();
-        testFinishFunction();
+        testCreateFinishFunctionPreflight(features);
+        testFinishWasmFailures(features);
+        testFinishFunction(features);
 
         // TODO: Update module with new host functions
-        testAllHostFunctions();
-        testKeyletHostFunctions();
+        testAllHostFunctions(features);
+        testKeyletHostFunctions(features);
     }
 
 public:

src/xrpld/app/tx/detail/ApplyContext.cpp

@@ -59,6 +59,7 @@ ApplyContext::discard()
 std::optional<TxMeta>
 ApplyContext::apply(TER ter)
 {
+    view_->setGasUsed(gasUsed_);
     return view_->apply(
         base_, tx, ter, parentBatchId_, flags_ & tapDRY_RUN, journal);
 }

src/xrpld/app/tx/detail/ApplyContext.h

@@ -108,9 +108,9 @@ public:
 
     /** Sets the gas used in the metadata */
     void
-    setGasUsed(std::uint32_t const& gasUsed)
+    setGasUsed(std::uint32_t const gasUsed)
     {
-        view_->setGasUsed(gasUsed);
+        gasUsed_ = gasUsed;
     }
 
     /** Discard changes and start fresh. */
@@ -164,6 +164,7 @@ private:
 
     // The ID of the batch transaction we are executing under, if seated.
     std::optional<uint256 const> parentBatchId_;
+    std::optional<std::uint32_t> gasUsed_;
 };
 
 }  // namespace ripple

src/xrpld/ledger/ApplyViewImpl.h

@@ -76,7 +76,7 @@ public:
     }
 
     void
-    setGasUsed(std::uint32_t const& gasUsed)
+    setGasUsed(std::optional<std::uint32_t> const gasUsed)
     {
         gasUsed_ = gasUsed;
     }