simplify fee logic, add extra check

include/xrpl/protocol/TER.h

@@ -69,6 +69,7 @@ enum TELcodes : TERUnderlyingType {
     telIMPORT_VL_KEY_NOT_RECOGNISED,
     telCAN_NOT_QUEUE_IMPORT,
     telENV_RPC_FAILED,
+    telMANIFEST_FEE_MISMATCH,
 };
 
 //------------------------------------------------------------------------------

src/libxrpl/protocol/TER.cpp

@@ -174,6 +174,7 @@ transResults()
         MAKE_ERROR(telIMPORT_VL_KEY_NOT_RECOGNISED, "Import vl key was not recognized."),
         MAKE_ERROR(telCAN_NOT_QUEUE_IMPORT,   "Import transaction was not able to be directly applied and cannot be queued."),
         MAKE_ERROR(telENV_RPC_FAILED,         "Unit test RPC failure."),
+        MAKE_ERROR(telMANIFEST_FEE_MISMATCH,  "Unsigned SetManifest fee is not, or cannot be confirmed as, canonical for this server's ledger fee settings."),
 
         MAKE_ERROR(temMALFORMED,                 "Malformed transaction."),
         MAKE_ERROR(temBAD_AMM_TOKENS,            "Malformed: Invalid LPTokens."),

src/test/app/SetHookTSH_test.cpp

@@ -8439,11 +8439,8 @@ private:
         using namespace std::literals;
         testcase("set manifest TSH");
 
-        if (!features[featureOnChainManifests])
-        {
-            pass();
-            return;
-        }
+        // OnChainManifests is not yet Supported::yes, so enable it explicitly.
+        features = features | featureOnChainManifests;
 
         // otxn: master
         // tsh: ephemeral

src/test/app/SetManifest_test.cpp

@@ -20,7 +20,9 @@
 #include <test/jtx.h>
 #include <test/jtx/network.h>
 #include <xrpld/app/ledger/OpenLedger.h>
+#include <xrpld/app/misc/HashRouter.h>
 #include <xrpld/app/misc/Manifest.h>
+#include <xrpld/app/misc/NetworkOPs.h>
 #include <xrpld/app/tx/apply.h>
 #include <xrpld/app/tx/detail/SetManifest.h>
 #include <xrpld/core/Config.h>
@@ -54,10 +56,10 @@ struct SetManifest_test : public beast::unit_test::suite
     // A manifest transaction is unsigned, so the network id is mandatory and
     // the network must be one that requires it (id > 1024).
     static std::unique_ptr<Config>
-    makeConfig()
+    makeConfig(std::string fee = "10")
     {
         return jtx::network::makeNetworkConfig(
-            21337, "10", "1000000", "200000");
+            21337, std::move(fee), "1000000", "200000");
     }
 
     /** Builds a manifest signed by `master` nominating `ephemeral`.
@@ -111,6 +113,15 @@ struct SetManifest_test : public beast::unit_test::suite
         return env.rpc("json", "submit", to_string(params))[jss::result];
     }
 
+    /** Submits an already formed SetManifest transaction. */
+    static Json::Value
+    submit(jtx::Env& env, std::shared_ptr<STTx const> const& tx)
+    {
+        Serializer s;
+        tx->add(s);
+        return env.rpc("submit", strHex(s.slice()))[jss::result];
+    }
+
     static std::string
     engineResult(Json::Value const& result)
     {
@@ -166,13 +177,13 @@ struct SetManifest_test : public beast::unit_test::suite
                 });
         };
 
-        // The same two-pass pricing makeSetManifestTx() does: encode once with
-        // a placeholder purely to have something to measure, then encode at the
-        // ceiling checkFee() will accept.
-        auto const base = SetManifest::calculateBaseFee(
-            *env.current(), *build(XRPAmount{0}, false));
-
-        return build(mulRatio(base, 12, 10, /*roundUp*/ true), true);
+        // The canonical Fee makeSetManifestTx() uses for the open ledger.
+        SerialIter mit{makeSlice(manifest)};
+        STObject const manifestObject{mit, sfManifest};
+        return build(
+            canonicalSetManifestFee(env.current()->fees().base, manifestObject)
+                .value_or(XRPAmount{0}),
+            true);
     }
 
     /** A mutable copy of a ledger object, suitable for the RawView interface.
@@ -597,7 +608,9 @@ struct SetManifest_test : public beast::unit_test::suite
         BEAST_EXPECT(
             applyDirect(env, envelope(env, good, other.id())) == temMALFORMED);
 
-        // Every envelope field a relayer could otherwise choose is pinned.
+        // Every envelope field a relayer could otherwise choose is pinned: the
+        // envelope is one exact transaction, with Account and NetworkID derived
+        // rather than mirrored.
         for (auto const& [name, tweak] : std::vector<
                  std::pair<char const*, std::function<void(STObject&)>>>{
                  {"Sequence",
@@ -608,33 +621,175 @@ struct SetManifest_test : public beast::unit_test::suite
                   }},
                  {"TicketSequence",
                   [](STObject& obj) { obj.setFieldU32(sfTicketSequence, 1); }},
-                 {"SigningPubKey", [&](STObject& obj) {
+                 {"SigningPubKey",
+                  [&](STObject& obj) {
                       obj.setFieldVL(sfSigningPubKey, master.pk().slice());
-                  }}})
+                  }},
+                 {"Memos",
+                  [](STObject& obj) {
+                      obj.setFieldArray(sfMemos, STArray(sfMemos, 1));
+                      STObject memo{sfMemo};
+                      memo.setFieldVL(sfMemoData, Blob{0x01});
+                      obj.peekFieldArray(sfMemos).emplace_back(std::move(memo));
+                  }},
+                 {"SourceTag",
+                  [](STObject& obj) { obj.setFieldU32(sfSourceTag, 1); }},
+                 {"LastLedgerSequence",
+                  [](STObject& obj) {
+                      obj.setFieldU32(sfLastLedgerSequence, 1'000'000);
+                  }},
+                 {"NetworkID",
+                  [](STObject& obj) { obj.setFieldU32(sfNetworkID, 21338); }}})
         {
+            auto const variant = envelope(env, good, master.id(), tweak);
+            // The generic network check runs first for a foreign NetworkID.
+            auto const expected = std::string(name) == "NetworkID"
+                ? TER{telWRONG_NETWORK}
+                : TER{temMALFORMED};
+            BEAST_EXPECTS(applyDirect(env, variant) == expected, name);
+
+            // Ingress rejects the variant as a bad envelope before any
+            // manifest signature work, and never as a soft fee miss.
+            auto const [validity, reason] = checkValidity(
+                env.app().getHashRouter(),
+                *variant,
+                env.current()->rules(),
+                env.app().config());
+            BEAST_EXPECTS(validity == Validity::SigBad, name);
             BEAST_EXPECTS(
-                applyDirect(env, envelope(env, good, master.id(), tweak)) ==
-                    temMALFORMED,
+                checkManifestIngressFee(env.app(), *variant).first ==
+                    ManifestIngressFee::NotApplicable,
                 name);
         }
 
-        // sfFee is the one envelope field preflight cannot bound, because the
-        // base fee is not in scope until preclaim. checkFee() caps it instead.
+        // The Fee is pinned to the one canonical value for the applying view's
+        // base fee. A different value is a local miss, not a malformed
+        // transaction, and ingress drops it before manifest crypto.
         auto const priced = envelope(env, good, master.id());
-        auto const ceiling = priced->getFieldAmount(sfFee).xrp();
+        auto const canonicalFee = priced->getFieldAmount(sfFee).xrp();
+        BEAST_EXPECT(
+            checkManifestIngressFee(env.app(), *priced).first ==
+            ManifestIngressFee::Canonical);
+        for (auto const fee :
+             {canonicalFee + XRPAmount{1}, canonicalFee - XRPAmount{1}})
+        {
+            auto const variant =
+                envelope(env, good, master.id(), [&](STObject& obj) {
+                    obj.setFieldAmount(sfFee, fee);
+                });
+            BEAST_EXPECT(
+                checkManifestIngressFee(env.app(), *variant).first ==
+                ManifestIngressFee::Refused);
+            BEAST_EXPECT(applyDirect(env, variant) == telMANIFEST_FEE_MISMATCH);
+        }
+
+        // At the canonical value exactly, which is what Submit sends.
+        BEAST_EXPECT(applyDirect(env, priced) == tesSUCCESS);
+    }
+
+    void
+    testCanonicalFee(FeatureBitset features)
+    {
+        testcase("canonical fee");
+        using namespace jtx;
+
+        auto const master = Account("master", KeyType::ed25519);
+        auto const eph1 = Account("eph1", KeyType::ed25519);
+        auto const units = [](STObject const& manifestObject) {
+            return 100 +
+                10 *
+                static_cast<std::int64_t>(
+                    manifestObject.getSerializer().getDataLength());
+        };
+        auto const manifest = makeManifest(master, eph1, 1);
+
+        // The builder prices against the voted base fee of the open ledger.
+        auto const build = [&](Env& env) -> std::shared_ptr<STTx const> {
+            auto const hex = makeSetManifestTx(
+                makeSlice(manifest),
+                env.app().config().NETWORK_ID,
+                *env.current(),
+                env.app().journal("SetManifest_test"));
+            if (!BEAST_EXPECT(hex))
+                return nullptr;
+            auto const bytes = strUnHex(*hex);
+            if (!BEAST_EXPECT(bytes))
+                return nullptr;
+            SerialIter txIter{makeSlice(*bytes)};
+            return std::make_shared<STTx const>(std::ref(txIter));
+        };
+        auto const manifestOf = [](STTx const& tx) -> STObject const& {
+            return const_cast<STTx&>(tx)
+                .getField(sfManifest)
+                .downcast<STObject>();
+        };
+
+        std::shared_ptr<STTx const> ordinaryTx;
+        {
+            Env ordinary{*this, makeConfig("10"), features};
+            ordinaryTx = build(ordinary);
+            if (!ordinaryTx)
+                return;
+            BEAST_EXPECT(
+                (*ordinaryTx)[sfFee].xrp().drops() ==
+                10 * units(manifestOf(*ordinaryTx)));
+
+            // A zero base would make the transaction free, so there is no
+            // canonical Fee for it at all.
+            BEAST_EXPECT(!canonicalSetManifestFee(
+                XRPAmount{0}, manifestOf(*ordinaryTx)));
+        }
+
+        // A different fee setting maps the same manifest to exactly one
+        // different txid.
+        Env expensive{*this, makeConfig("100000"), features};
+        auto const expensiveTx = build(expensive);
+        if (!expensiveTx)
+            return;
+        BEAST_EXPECT(
+            (*expensiveTx)[sfFee].xrp().drops() ==
+            100'000 * units(manifestOf(*expensiveTx)));
+        BEAST_EXPECT(
+            expensiveTx->getTransactionID() != ordinaryTx->getTransactionID());
+
+        expensive.fund(XRP(10000), master);
+        expensive.close();
+
+        // A wrapper priced for another fee setting is a local miss, not a bad
+        // signature: ingress drops it before manifest crypto without marking
+        // it bad, and preclaim refuses it with a tel result.
+        BEAST_EXPECT(
+            checkManifestIngressFee(expensive.app(), *ordinaryTx).first ==
+            ManifestIngressFee::Refused);
+        BEAST_EXPECT(
+            submit(expensive, ordinaryTx)[jss::error] == "invalidTransaction");
+        BEAST_EXPECT(
+            (expensive.app().getHashRouter().getFlags(
+                 ordinaryTx->getTransactionID()) &
+             SF_BAD) == 0);
+        BEAST_EXPECT(
+            applyDirect(expensive, ordinaryTx) == telMANIFEST_FEE_MISMATCH);
 
         BEAST_EXPECT(
-            applyDirect(
-                env, envelope(env, good, master.id(), [&](STObject& obj) {
-                    obj.setFieldAmount(sfFee, ceiling + XRPAmount{1});
-                })) == temBAD_FEE);
+            checkManifestIngressFee(expensive.app(), *expensiveTx).first ==
+            ManifestIngressFee::Canonical);
+        BEAST_EXPECT(engineResult(submit(expensive, manifest)) == "tesSUCCESS");
+        expensive.close();
+        BEAST_EXPECT(expensive.le(keylet::manifest(master.pk())));
 
-        // At the ceiling exactly, which is what Submit sends.
+        // An amendment-blocked server has no base fee it can trust, so ingress
+        // refuses even the canonical transaction rather than verify it.
+        Env blocked{*this, makeConfig("10"), features};
+        auto const blockedTx = build(blocked);
+        if (!blockedTx)
+            return;
         BEAST_EXPECT(
-            applyDirect(
-                env, envelope(env, good, master.id(), [&](STObject& obj) {
-                    obj.setFieldAmount(sfFee, ceiling);
-                })) == tesSUCCESS);
+            checkManifestIngressFee(blocked.app(), *blockedTx).first ==
+            ManifestIngressFee::Canonical);
+        blocked.app().getOPs().setAmendmentBlocked();
+        BEAST_EXPECT(
+            checkManifestIngressFee(blocked.app(), *blockedTx).first ==
+            ManifestIngressFee::Refused);
     }
 
     void
@@ -807,6 +962,7 @@ public:
         testSigningKeyRetrieval(sa);
         testMalformed(sa);
         testEnvelopeRejections(sa);
+        testCanonicalFee(sa);
         testCorruptLedger(sa);
         testGossipSelection(sa);
         testDisabled(sa);

src/xrpld/app/misc/NetworkOPs.cpp

@@ -1136,6 +1136,16 @@ NetworkOPsImp::submitTransaction(std::shared_ptr<STTx const> const& iTrans)
 
     try
     {
+        // Not a bad signature: this server may not have reached the fee
+        // setting the transaction was built for. Drop without marking it bad.
+        if (auto const [fee, feeReason] = checkManifestIngressFee(app_, *trans);
+            fee == ManifestIngressFee::Refused)
+        {
+            JLOG(m_journal.debug())
+                << "Submitted manifest-authorized tx dropped: " << feeReason;
+            return;
+        }
+
         auto const [validity, reason] = checkValidity(
             app_.getHashRouter(),
             *trans,
@@ -1273,6 +1283,19 @@ NetworkOPsImp::processTransaction(
     // NOTE eahennis - I think this check is redundant,
     // but I'm not 100% sure yet.
     // If so, only cost is looking up HashRouter flags.
+    // Before checkValidity(), which verifies the manifest: a fee miss is local
+    // and must not mark the transaction bad.
+    if (auto const [fee, feeReason] =
+            checkManifestIngressFee(app_, *transaction->getSTransaction());
+        fee == ManifestIngressFee::Refused)
+    {
+        JLOG(m_journal.debug())
+            << "Manifest-authorized tx dropped: " << feeReason;
+        transaction->setStatus(INVALID);
+        transaction->setResult(telMANIFEST_FEE_MISMATCH);
+        return;
+    }
+
     auto const [validity, reason] = checkValidity(
         app_.getHashRouter(),
         *transaction->getSTransaction(),

src/xrpld/app/tx/apply.h

@@ -68,6 +68,42 @@ checkValidity(
     Config const& config,
     ApplyFlags const flags = tapNONE);
 
+/** Ingress fee outcome for a manifest-authorized SetManifest.
+
+    @see checkManifestIngressFee
+*/
+enum class ManifestIngressFee {
+    /// Not a manifest-authorized SetManifest, or the amendment is not
+    /// enabled; checkValidity applies as usual.
+    NotApplicable,
+    /// The Fee is canonical for the ledger this server would apply it to.
+    Canonical,
+    /// A canonically shaped envelope whose Fee cannot be confirmed here. Drop
+    /// without verifying the manifest. Not a bad signature: the same bytes
+    /// may be canonical on another node, or here once this node reaches the
+    /// fee setting they were built for.
+    Refused
+};
+
+/** Checks an unsigned SetManifest's Fee before any manifest signature work.
+
+    The canonical Fee follows the voted base fee of the parent of the ledger
+    a transaction is applied to. Ingress accepts exactly the value for this
+    server's open ledger, priced from its parent: the same value preclaim
+    enforces, so nothing that passes here is refused there for its Fee. A
+    server with no validated ledger, or one that is amendment blocked, has no
+    trustworthy base and refuses.
+
+    Every ingress caller runs this before checkValidity(), which no longer
+    checks the Fee because the transactor engine calls it without a ledger.
+    A Refused result must be dropped without marking the transaction bad or
+    charging for an invalid signature.
+
+    @return the outcome and, if Refused, a reason
+*/
+std::pair<ManifestIngressFee, std::string>
+checkManifestIngressFee(Application& app, STTx const& tx);
+
 /** Sets the validity of a given transaction in the cache.
 
     @warning Use with extreme care.

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

@@ -26,14 +26,126 @@
 #include <xrpl/protocol/Feature.h>
 #include <xrpl/protocol/Indexes.h>
 #include <xrpl/protocol/PublicKey.h>
-#include <xrpl/protocol/Quality.h>
+#include <xrpl/protocol/SystemParameters.h>
 #include <xrpl/protocol/TxFlags.h>
-#include <xrpl/protocol/XRPAmount.h>  // mulRatio
+#include <xrpl/protocol/XRPAmount.h>
 #include <xrpl/protocol/serialize.h>
 #include <xrpl/protocol/st.h>
 
+#include <limits>
+
 namespace ripple {
 
+bool
+hasManifestAuthorityMarkers(STTx const& tx) noexcept
+{
+    try
+    {
+        return tx.getTxnType() == ttMANIFEST_SET &&
+            tx.isFieldPresent(sfSigningPubKey) &&
+            tx.getSigningPubKey().empty() &&
+            tx.isFieldPresent(sfTxnSignature) && tx.getSignature().empty();
+    }
+    catch (std::exception const&)
+    {
+        return false;
+    }
+}
+
+/** Build the only permitted outer transaction for `manifest`. */
+static STTx
+canonicalSetManifest(
+    STObject const& manifest,
+    AccountID const& account,
+    std::optional<std::uint32_t> networkID,
+    XRPAmount fee)
+{
+    return STTx(ttMANIFEST_SET, [&](STObject& obj) {
+        obj.setAccountID(sfAccount, account);
+        obj.setFieldU32(sfSequence, 0);
+        if (networkID)
+            obj.setFieldU32(sfNetworkID, *networkID);
+        obj.setFieldAmount(sfFee, fee);
+        obj.setFieldVL(sfSigningPubKey, Blob{});
+        obj.setFieldVL(sfTxnSignature, Blob{});
+
+        obj.peekFieldObject(sfManifest) = manifest;
+    });
+}
+
+/** The NetworkID a canonical envelope carries for this server's network.
+
+    Networks above 1024 require the field and legacy networks forbid it
+    (preflight0), so the canonical envelope follows the same rule.
+ */
+static std::optional<std::uint32_t>
+canonicalNetworkID(std::uint32_t networkID)
+{
+    if (networkID > 1024)
+        return networkID;
+    return std::nullopt;
+}
+
+bool
+hasCanonicalSetManifestShape(STTx const& tx, std::uint32_t networkID) noexcept
+{
+    try
+    {
+        auto const& manifest =
+            const_cast<STTx&>(tx).getField(sfManifest).downcast<STObject>();
+        auto const masterKey = manifest.getFieldVL(sfPublicKey);
+        if (!publicKeyType(makeSlice(masterKey)))
+            return false;
+
+        auto const canonical = canonicalSetManifest(
+            manifest,
+            calcAccountID(PublicKey(makeSlice(masterKey))),
+            canonicalNetworkID(networkID),
+            tx[sfFee].xrp());
+
+        auto const actualBytes = tx.getSerializer();
+        auto const canonicalBytes = canonical.getSerializer();
+        return actualBytes.slice() == canonicalBytes.slice();
+    }
+    catch (std::exception const&)
+    {
+        return false;
+    }
+}
+
+std::optional<XRPAmount>
+canonicalSetManifestFee(XRPAmount base, STObject const& manifest)
+{
+    // Every relayer must compute the same Fee, so it depends only on the voted
+    // reference fee (ledger state, never local load) and the manifest's size.
+    // Only the multipliers are fixed; a fee vote changes the canonical txid
+    // once.
+    constexpr std::int64_t baseUnits = 100;
+    constexpr std::int64_t unitsPerByte = 10;
+    constexpr auto maxDrops = std::numeric_limits<std::int64_t>::max();
+
+    // Largest size for which baseUnits + bytes * unitsPerByte cannot overflow.
+    constexpr std::uint64_t maxBytes = (maxDrops - baseUnits) / unitsPerByte;
+
+    auto const bytes = manifest.getSerializer().getDataLength();
+
+    // A non-positive base would make the transaction free.
+    if (base <= beast::zero || bytes == 0 || bytes > maxBytes)
+        return std::nullopt;
+
+    auto const units =
+        baseUnits + static_cast<std::int64_t>(bytes) * unitsPerByte;
+
+    // base is positive, so this is an exact test for units * base overflowing.
+    if (units > maxDrops / base.drops())
+        return std::nullopt;
+
+    XRPAmount const fee{base.drops() * units};
+    if (!isLegalAmount(fee) || fee <= beast::zero)
+        return std::nullopt;
+    return fee;
+}
+
 TxConsequences
 SetManifest::makeTxConsequences(PreflightContext const& ctx)
 {
@@ -58,6 +170,19 @@ SetManifest::preflight(PreflightContext const& ctx)
         return temINVALID_FLAG;
     }
 
+    // The envelope carries no account signature: authority comes solely from
+    // the manifest's own master and ephemeral signatures, which do not cover
+    // it. So the whole envelope is one exact transaction per manifest, with
+    // Account derived from the manifest's master key and NetworkID from this
+    // server, and only the Fee mirrored here (checkFee() pins it). Checked
+    // before any signature work, so a relayer cannot vary a field to mint
+    // fresh txids that each cost manifest signature checks.
+    if (!hasCanonicalSetManifestShape(tx, ctx.app.config().NETWORK_ID))
+    {
+        JLOG(j.warn()) << "SetManifest: non-canonical envelope.";
+        return temMALFORMED;
+    }
+
     // rules:
     // 1. sfManifest must match the manifest template and be validly signed
     // 2. the signingpubkey must match the master r-address
@@ -95,23 +220,6 @@ SetManifest::preflight(PreflightContext const& ctx)
     // 3. not already revoked will be checked in preclaim because it depends on
     // lgr state
 
-    // 4. the envelope carries no account signature: authority comes solely
-    // from the manifest's own master/ephemeral signatures, which do not cover
-    // the envelope. Pin every envelope field a relayer could otherwise choose.
-    // The shape below must match the one checkValidity() recognises, or the
-    // txn falls through to the ordinary signature path and is rejected there.
-    // sfFee cannot be bounded here because the computed base fee is not in
-    // scope until preclaim; checkFee() bounds it instead.
-    if (!tx.isFieldPresent(sfSigningPubKey) || !tx.getSigningPubKey().empty() ||
-        !tx.isFieldPresent(sfTxnSignature) || !tx.getSignature().empty() ||
-        tx.isFieldPresent(sfSigners) || tx.isFieldPresent(sfAccountTxnID) ||
-        tx.isFieldPresent(sfTicketSequence) || tx.getFieldU32(sfSequence) != 0)
-    {
-        JLOG(j.warn())
-            << "SetManifest: envelope must be unsigned with Sequence 0.";
-        return temMALFORMED;
-    }
-
     return preflight2(ctx);
 }
 
@@ -137,9 +245,9 @@ SetManifest::preclaim(PreclaimContext const& ctx)
         return tefINTERNAL;  // preflight already parsed this successfully
 
     // Replay protection. A byte-identical resubmission is rejected as
-    // tefALREADY by checkPriorTxAndLastLedger, but sfFee may vary within the
-    // band checkFee() allows, so the same manifest can also arrive under a
-    // different txid. The strictly-increasing sequence test below is what
+    // tefALREADY by checkPriorTxAndLastLedger, but the canonical Fee follows
+    // the voted base fee, so the same manifest can arrive under a different
+    // txid after a fee change. The strictly-increasing sequence test below
     // covers that, both within this ledger and in every later one. Either
     // result is tef, so a replay is never included and never claims a fee.
     if (sle->isFieldPresent(sfManifestID))
@@ -312,33 +420,26 @@ SetManifest::calculateBaseFee(ReadView const& view, STTx const& tx)
     return Transactor::calculateBaseFee(view, tx) + manifestFee;
 }
 
-/** The most sfFee may be: the same 1.2x headroom Submit applies.
-
-    Kept in one place so the value Submit writes and the value preclaim will
-    accept cannot drift apart.
-*/
-static XRPAmount
-manifestFeeCeiling(XRPAmount baseFee)
-{
-    return mulRatio(baseFee, 12, 10, /*roundUp*/ true);
-}
-
 TER
 SetManifest::checkFee(PreclaimContext const& ctx, XRPAmount baseFee)
 {
-    // A ceiling is required because the envelope carries no account signature,
-    // so sfFee is chosen by whoever relays the txn -- and manifests are public:
-    // they are gossiped over the peer protocol and embedded in published UNLs,
-    // so the relayer need not be the master key holder. Uncapped, any observer
-    // of a not-yet-recorded manifest could wrap it with sfFee set to that
-    // validator's entire balance. The 20% band is headroom against a fee floor
-    // that has risen since the txn was built, and bounds what an attacker can
-    // burn to the same 20%.
-    if (ctx.tx[sfFee].xrp() > manifestFeeCeiling(baseFee))
+    // The envelope carries no account signature, so a relayer-chosen Fee would
+    // be both a malleable field and a way to charge the master account. The
+    // canonical Fee is priced from this view's voted base fee, which is the
+    // parent ledger's: one exact value per fee setting. A mismatch is local,
+    // not final: the same bytes are canonical on a server that has reached
+    // the fee setting they were built for. The check also runs on a closed
+    // ledger, so a proposer cannot charge the master account an arbitrary Fee
+    // by skipping it.
+    STObject const& manifest =
+        const_cast<STTx&>(ctx.tx).getField(sfManifest).downcast<STObject>();
+    auto const canonical =
+        canonicalSetManifestFee(ctx.view.fees().base, manifest);
+    if (!canonical || ctx.tx[sfFee].xrp() != *canonical)
     {
-        JLOG(ctx.j.trace()) << "SetManifest: fee above ceiling: "
+        JLOG(ctx.j.trace()) << "SetManifest: non-canonical fee: "
                             << to_string(ctx.tx[sfFee].xrp());
-        return temBAD_FEE;
+        return telMANIFEST_FEE_MISMATCH;
     }
 
     // Floor and balance are the ordinary rules.
@@ -358,41 +459,19 @@ makeSetManifestTx(
         if (!man || !man->verify())
             return std::nullopt;
 
-        auto const encode = [&](XRPAmount fee) {
-            return serializeHex(STTx(ttMANIFEST_SET, [&](STObject& obj) {
-                obj.setAccountID(sfAccount, calcAccountID(man->masterKey));
-                obj.setFieldU32(sfSequence, 0);
-                obj.setFieldU32(sfNetworkID, networkID);
-                obj.setFieldAmount(sfFee, fee);
-                obj.setFieldVL(sfSigningPubKey, std::vector<std::uint8_t>{});
-                obj.setFieldVL(sfTxnSignature, std::vector<std::uint8_t>{});
+        SerialIter manifestIter{manifest};
+        STObject const manifestObject{manifestIter, sfManifest};
 
-                // sfManifest is soeREQUIRED, so STObject::set(SOTemplate) has
-                // already materialised it as a present, empty object. Fill
-                // that one in: emitting a second is a duplicate field, which
-                // STObject::set(SerialIter&) rejects on the way back in.
-                SerialIter mit{manifest};
-                obj.peekFieldObject(sfManifest).set(mit);
-            }));
-        };
-
-        // calculateBaseFee() takes a parsed transaction, so encode once with a
-        // placeholder fee purely to have something to price. The resulting fee
-        // does not depend on the placeholder: it is derived from the length of
-        // the manifest object and the ledger's base fee.
-        auto const priced = strUnHex(encode(XRPAmount{0}));
-        if (!priced || priced->empty())
+        auto const fee =
+            canonicalSetManifestFee(openView.fees().base, manifestObject);
+        if (!fee)
             return std::nullopt;
 
-        SerialIter sit{makeSlice(*priced)};
-        STTx const probe{std::ref(sit)};
-
-        // Submit the ceiling exactly. preclaim rejects anything above it, and
-        // the floor rises with network load, so the ceiling is both always
-        // acceptable and the value most likely to still clear the floor by the
-        // time the transaction is applied.
-        return encode(
-            manifestFeeCeiling(SetManifest::calculateBaseFee(openView, probe)));
+        return serializeHex(canonicalSetManifest(
+            manifestObject,
+            calcAccountID(man->masterKey),
+            canonicalNetworkID(networkID),
+            *fee));
     }
     catch (std::exception const& e)
     {

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

@@ -25,29 +25,54 @@
 #include <xrpl/basics/Log.h>
 #include <xrpl/protocol/Indexes.h>
 
+#include <optional>
+
 namespace ripple {
 
+/** Return whether empty outer signatures nominate manifest-only authority. */
+bool
+hasManifestAuthorityMarkers(STTx const& tx) noexcept;
+
+/** Return whether every outer byte has the canonical SetManifest shape.
+
+    Account is derived from the manifest's master key and NetworkID from
+    `networkID`, this server's network. Only the Fee is mirrored from the
+    candidate; checkFee pins it against the applying view.
+*/
+bool
+hasCanonicalSetManifestShape(STTx const& tx, std::uint32_t networkID) noexcept;
+
+/** Return the one canonical Fee for a SetManifest carrying `manifest`.
+
+    The Fee is `base * (100 + 10 * manifestBytes)`, where `base` is the voted
+    reference fee of the ledger the transaction is checked against, which is
+    its parent's. Every node reading the same ledger computes the same value,
+    so one manifest maps to one transaction ID per fee setting; local load
+    never enters it.
+
+    Returns nullopt when there is no canonical Fee: a zero base, or a product
+    that overflows or is not a legal amount.
+*/
+std::optional<XRPAmount>
+canonicalSetManifestFee(XRPAmount base, STObject const& manifest);
+
 /** Encode the transaction that publishes `manifest` on-ledger.
 
-    A manifest transaction carries no account signature, so the protocol pins
-    the whole envelope: Sequence must be 0, SigningPubKey and TxnSignature must
-    be empty, and Fee must fall between the computed base fee and a ceiling
-    above it. SetManifest::preflight and SetManifest::checkFee reject anything
-    else. Every caller that submits a manifest builds it here so those rules
-    cannot drift apart from the ones the transactor enforces.
-
-    Returns hex rather than an STTx because the manifest is appended to the
-    encoded transaction verbatim, behind its object marker, instead of being
-    parsed and re-emitted: the bytes the master key signed survive untouched,
-    and a future change to the manifest format needs no change here.
+    A manifest transaction carries no account signature, so the whole
+    envelope is one exact transaction: Account is the manifest master key's
+    account, Sequence is 0, SigningPubKey and TxnSignature are empty, NetworkID
+    is this network's, and Fee is the canonical Fee for `openView`'s base fee.
+    No other field is allowed. SetManifest::preflight and SetManifest::checkFee
+    reject anything else, and every caller that submits a manifest builds it
+    here.
 
     @param manifest Serialized manifest
     @param networkID Network the transaction is for
-    @param openView Ledger the fee is priced against
+    @param openView Open ledger the Fee is priced against
     @param j Journal
 
     @return the hex-encoded transaction, or nullopt if the manifest does not
-        parse or does not verify
+        parse or does not verify, or there is no canonical Fee
 */
 std::optional<std::string>
 makeSetManifestTx(

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

@@ -17,10 +17,15 @@
 */
 //==============================================================================
 
+#include <xrpld/app/ledger/LedgerMaster.h>
+#include <xrpld/app/ledger/OpenLedger.h>
+#include <xrpld/app/main/Application.h>
 #include <xrpld/app/misc/HashRouter.h>
 #include <xrpld/app/misc/Manifest.h>
+#include <xrpld/app/misc/NetworkOPs.h>
 #include <xrpld/app/tx/apply.h>
 #include <xrpld/app/tx/applySteps.h>
+#include <xrpld/app/tx/detail/SetManifest.h>
 #include <xrpl/basics/Log.h>
 #include <xrpl/protocol/Feature.h>
 
@@ -75,12 +80,17 @@ checkValidity(
     }
 
     if (rules.enabled(featureOnChainManifests) &&
-        tx.getTxnType() == ttMANIFEST_SET &&
-        tx.isFieldPresent(sfTxnSignature) &&
-        tx.getFieldVL(sfTxnSignature).empty() &&
-        tx.isFieldPresent(sfSigningPubKey) &&
-        tx.getFieldVL(sfSigningPubKey).empty() && tx.isFieldPresent(sfManifest))
+        hasManifestAuthorityMarkers(tx) && tx.isFieldPresent(sfManifest))
     {
+        // Structural nonsense must not buy manifest signature checks. The Fee
+        // depends on ledger fee state, which this function does not have;
+        // ingress callers pin it first with checkManifestIngressFee(), and the
+        // transactor pins it against the applying view in checkFee().
+        if (!hasCanonicalSetManifestShape(tx, config.NETWORK_ID))
+            return {
+                Validity::SigBad,
+                "Manifest-authorized envelope is not canonical"};
+
         // perform alternative signature check over manifest
         STObject const& manObj = const_cast<ripple::STTx&>(tx)
                                      .getField(sfManifest)
@@ -221,4 +231,56 @@ applyTransaction(
     }
 }
 
+std::pair<ManifestIngressFee, std::string>
+checkManifestIngressFee(Application& app, STTx const& tx)
+{
+    auto const open = app.openLedger().current();
+    if (!open->rules().enabled(featureOnChainManifests) ||
+        !hasManifestAuthorityMarkers(tx))
+        return {ManifestIngressFee::NotApplicable, {}};
+
+    // A non-canonical shape can never become valid, so it is not a fee miss:
+    // leave it to checkValidity(), which rejects it as a bad envelope.
+    if (!hasCanonicalSetManifestShape(tx, app.config().NETWORK_ID))
+        return {ManifestIngressFee::NotApplicable, {}};
+
+    // An amendment-blocked node's open ledger may sit on history the network
+    // is not validating, and with no validated ledger there is no agreed base
+    // at all. Neither is a reason to spend manifest crypto finding out.
+    if (app.getOPs().isAmendmentBlocked())
+        return {ManifestIngressFee::Refused, "server is amendment blocked"};
+    auto const validated = app.getLedgerMaster().getValidatedLedger();
+    if (!validated)
+        return {ManifestIngressFee::Refused, "no validated ledger"};
+
+    try
+    {
+        auto const fee = tx[sfFee].xrp();
+        auto const& manifest =
+            const_cast<STTx&>(tx).getField(sfManifest).downcast<STObject>();
+
+        // A transaction is priced from the parent of the ledger it is applied
+        // to. Here that is the open ledger, whose fees() are its parent's:
+        // exactly what preclaim enforces. A wrapper priced for any other fee
+        // setting cannot apply on this server, so it is not worth verifying,
+        // and no remembered setting widens the set of txids that reach
+        // manifest signature checks.
+        if (auto const canonical =
+                canonicalSetManifestFee(open->fees().base, manifest);
+            canonical && *canonical == fee)
+            return {ManifestIngressFee::Canonical, {}};
+    }
+    catch (std::exception const&)
+    {
+        // Malformed Fee or manifest: leave it to checkValidity()'s shape
+        // check, which rejects it as a bad envelope.
+        return {ManifestIngressFee::NotApplicable, {}};
+    }
+
+    return {
+        ManifestIngressFee::Refused,
+        "Fee is not the canonical SetManifest fee for this server's open "
+        "ledger"};
+}
+
 }  // namespace ripple

src/xrpld/overlay/detail/PeerImp.cpp

@@ -2878,6 +2878,20 @@ PeerImp::checkTransaction(
             }
         }
 
+        // A manifest-authorized SetManifest's Fee follows the voted base fee,
+        // so a miss may be a fee setting this server has not reached yet.
+        // Drop it before any manifest signature work, but do not mark it bad:
+        // the same bytes can become canonical here.
+        if (auto const [fee, feeReason] = checkManifestIngressFee(app_, *stx);
+            fee == ManifestIngressFee::Refused)
+        {
+            JLOG(p_journal_.trace())
+                << "Dropping manifest-authorized tx " << stx->getTransactionID()
+                << ": " << feeReason;
+            charge(Resource::feeUselessData, "manifest fee miss");
+            return;
+        }
+
         if (checkSignature)
         {
             // Check the signature before handing off to the job queue.

src/xrpld/rpc/handlers/Submit.cpp

@@ -197,6 +197,18 @@ doSubmit(RPC::JsonContext& context)
     }
 
     {
+        // Before any manifest signature work. Not a bad signature, so the
+        // transaction is not marked bad.
+        if (auto const [fee, feeReason] =
+                checkManifestIngressFee(context.app, *stTx);
+            fee == ManifestIngressFee::Refused)
+        {
+            jvResult[jss::error] = "invalidTransaction";
+            jvResult[jss::error_exception] = "fails local checks: " + feeReason;
+
+            return jvResult;
+        }
+
         if (!context.app.checkSigs())
             forceValidity(
                 context.app.getHashRouter(),