fix: uritoken destination & amount preflight check (#188)

* fix: uritoken destination & amount

* Update URIToken.cpp

* add lsfBurnable flag

* make uritoken patch a fix amendment

* clang-format

CMakeLists.txt

@@ -45,7 +45,6 @@ include(RippledSanity)
 include(RippledVersion)
 include(RippledSettings)
 include(RippledNIH)
-
 # this check has to remain in the top-level cmake
 # because of the early return statement
 if (packages_only)

src/ripple/app/hook/impl/applyHook.cpp

@@ -135,7 +135,7 @@ getTransactionalStakeHolders(STTx const& tx, ReadView const& rv)
             // issuer is also a strong TSH if the burnable flag is set
             auto const issuer = ut->getAccountID(sfIssuer);
             if (issuer != owner)
-                ADD_TSH(issuer, ut->getFlags() & tfBurnable);
+                ADD_TSH(issuer, ut->getFlags() & lsfBurnable);
 
             break;
         }
@@ -154,7 +154,7 @@ getTransactionalStakeHolders(STTx const& tx, ReadView const& rv)
 
             // issuer is a strong TSH if the burnable flag is set
             if (issuer != owner)
-                ADD_TSH(issuer, ut->getFlags() & tfBurnable);
+                ADD_TSH(issuer, ut->getFlags() & lsfBurnable);
 
             // destination is a strong tsh
             if (tx.isFieldPresent(sfDestination))

src/ripple/app/tx/impl/URIToken.cpp

@@ -78,6 +78,17 @@ URIToken::preflight(PreflightContext const& ctx)
         }
     }
 
+    // fix amendment to return temMALFORMED if sfDestination field is present
+    // and sfAmount field is not present
+    if (ctx.rules.enabled(fixURITokenV1))
+    {
+        if (ctx.tx.isFieldPresent(sfDestination) &&
+            !ctx.tx.isFieldPresent(sfAmount))
+        {
+            return temMALFORMED;
+        }
+    }
+
     // the validation for the URI field is also the same regardless of the txn
     // type
     if (ctx.tx.isFieldPresent(sfURI))
@@ -231,7 +242,7 @@ URIToken::preclaim(PreclaimContext const& ctx)
         }
 
         case ttURITOKEN_BURN: {
-            if (leFlags == tfBurnable && acc == *issuer)
+            if (leFlags == lsfBurnable && acc == *issuer)
             {
                 // pass, the issuer can burn the URIToken if they minted it with
                 // a burn flag
@@ -805,9 +816,9 @@ URIToken::doApply()
             }
             else if (
                 sleU->getAccountID(sfIssuer) == account_ &&
-                (sleU->getFlags() & tfBurnable))
+                (sleU->getFlags() & lsfBurnable))
             {
-                // pass, issuer may burn if the tfBurnable flag was set during
+                // pass, issuer may burn if the lsfBurnable flag was set during
                 // minting
             }
             else

src/ripple/protocol/Feature.h

@@ -74,7 +74,7 @@ namespace detail {
 // Feature.cpp. Because it's only used to reserve storage, and determine how
 // large to make the FeatureBitset, it MAY be larger. It MUST NOT be less than
 // the actual number of amendments. A LogicError on startup will verify this.
-static constexpr std::size_t numFeatures = 65;
+static constexpr std::size_t numFeatures = 66;
 
 /** Amendments that this server supports and the default voting behavior.
    Whether they are enabled depends on the Rules defined in the validated
@@ -353,6 +353,7 @@ extern uint256 const fixNFTokenRemint;
 extern uint256 const featureImport;
 extern uint256 const featureXahauGenesis;
 extern uint256 const featureHooksUpdate1;
+extern uint256 const fixURITokenV1;
 
 }  // namespace ripple
 

src/ripple/protocol/LedgerFormats.h

@@ -310,6 +310,9 @@ enum LedgerSpecificFlags {
 
     // ltNFTOKEN_OFFER
     lsfSellNFToken = 0x00000001,
+
+    // ltURI_TOKEN
+    lsfBurnable = 0x00000001,  // True, issuer can burn the token
 };
 
 //------------------------------------------------------------------------------

src/ripple/protocol/impl/Feature.cpp

@@ -459,6 +459,7 @@ REGISTER_FEATURE(URIToken,                      Supported::yes, VoteBehavior::De
 REGISTER_FEATURE(Import,                        Supported::yes, VoteBehavior::DefaultYes);
 REGISTER_FEATURE(XahauGenesis,                  Supported::yes, VoteBehavior::DefaultYes);
 REGISTER_FEATURE(HooksUpdate1,                  Supported::yes, VoteBehavior::DefaultYes);
+REGISTER_FIX    (fixURITokenV1,                 Supported::yes, VoteBehavior::DefaultNo);
 
 
 // The following amendments are obsolete, but must remain supported

src/test/app/URIToken_test.cpp

@@ -260,6 +260,34 @@ struct URIToken_test : public beast::unit_test::suite
         using namespace jtx;
         using namespace std::literals::chrono_literals;
 
+        // fixURITokenV1
+        {
+            for (bool const withFixURITokenV1 : {true, false})
+            {
+                auto const amend =
+                    withFixURITokenV1 ? features : features - fixURITokenV1;
+
+                auto const txResult =
+                    withFixURITokenV1 ? ter(temMALFORMED) : ter(tefINTERNAL);
+
+                Env env{*this, amend};
+                auto const alice = Account("alice");
+                auto const bob = Account("bob");
+                env.fund(XRP(1000), alice, bob);
+                env.close();
+
+                std::string const uri(2, '?');
+                auto const tid = tokenid(alice, uri);
+                std::string const hexid{strHex(tid)};
+
+                // temMALFORMED - cannot include sfDestination without sfAmount
+                Json::Value destTx = mint(alice, uri);
+                destTx[jss::Destination] = bob.human();
+                env(destTx, txResult);
+                env.close();
+            }
+        }
+
         // setup env
         Env env{*this, features};
         auto const alice = Account("alice");

src/test/rpc/LedgerRPC_test.cpp

@@ -1705,7 +1705,7 @@ public:
                 "json", "ledger_entry", to_string(jvParams))[jss::result];
             BEAST_EXPECT(jrr[jss::node][sfOwner.jsonName] == alice.human());
             BEAST_EXPECT(jrr[jss::node][sfURI.jsonName] == strHex(uri));
-            BEAST_EXPECT(jrr[jss::node][sfFlags.jsonName] == 1);
+            BEAST_EXPECT(jrr[jss::node][sfFlags.jsonName] == lsfBurnable);
         }
         {
             // Request an index that is not a uritoken.