Merge branch 'dev' into ammLPHolds-iouescrow

build-core.sh

@@ -95,16 +95,8 @@ if [[ "$4" == "" ]]; then
   echo "Non GH, local building, no Action runner magic"
 else
   # GH Action, runner
-    if [[ "$(git rev-parse --abbrev-ref HEAD)" == "release" ]]; then
-        echo "building on the release branch... placing it in builds/candidate"
-        mkdir /data/builds/candidate
-        cp /io/release-build/xahaud /data/builds/candidate/$(date +%Y).$(date +%-m).$(date +%-d)-$(git rev-parse --abbrev-ref HEAD)+$4
-        cp /io/release-build/release.info /data/builds/candidate/$(date +%Y).$(date +%-m).$(date +%-d)-$(git rev-parse --abbrev-ref HEAD)+$4.releaseinfo
-    else
-        echo "building non-release branch, placing it in builds root"
-        cp /io/release-build/xahaud /data/builds/$(date +%Y).$(date +%-m).$(date +%-d)-$(git rev-parse --abbrev-ref HEAD)+$4
-        cp /io/release-build/release.info /data/builds/$(date +%Y).$(date +%-m).$(date +%-d)-$(git rev-parse --abbrev-ref HEAD)+$4.releaseinfo
-    fi  
+  cp /io/release-build/xahaud /data/builds/$(date +%Y).$(date +%-m).$(date +%-d)-$(git rev-parse --abbrev-ref HEAD)+$4
+  cp /io/release-build/release.info /data/builds/$(date +%Y).$(date +%-m).$(date +%-d)-$(git rev-parse --abbrev-ref HEAD)+$4.releaseinfo
   echo "Published build to: http://build.xahau.tech/"
   echo $(date +%Y).$(date +%-m).$(date +%-d)-$(git rev-parse --abbrev-ref HEAD)+$4
 fi

include/xrpl/protocol/detail/features.macro

@@ -34,8 +34,6 @@
 // If you add an amendment here, then do not forget to increment `numFeatures`
 // in include/xrpl/protocol/Feature.h.
 
-XRPL_FEATURE(SecureUI,                   Supported::yes, VoteBehavior::DefaultNo)
-XRPL_FIX    (HookMap,                    Supported::yes, VoteBehavior::DefaultYes)
 XRPL_FIX    (GuardDepth32,               Supported::yes, VoteBehavior::DefaultNo)
 XRPL_FEATURE(NamedHooks,                 Supported::yes, VoteBehavior::DefaultNo)
 XRPL_FEATURE(IOURewardClaim,             Supported::yes, VoteBehavior::DefaultNo)

include/xrpl/protocol/detail/ledger_entries.macro

@@ -262,7 +262,6 @@ LEDGER_ENTRY(ltACCOUNT_ROOT, 0x0061, AccountRoot, account, ({
     {sfHookStateScale,       soeOPTIONAL},
     {sfCron,                 soeOPTIONAL},
     {sfAMMID,                soeOPTIONAL},
-    {sfSecureUI,             soeOPTIONAL},
 }))
 
 /** A ledger object which contains a list of object identifiers.

include/xrpl/protocol/detail/sfields.macro

@@ -293,7 +293,6 @@ TYPED_SFIELD(sfAssetClass,               VL,        29)
 TYPED_SFIELD(sfProvider,                 VL,        30)
 TYPED_SFIELD(sfMPTokenMetadata,          VL,        31)
 TYPED_SFIELD(sfCredentialType,           VL,        32)
-TYPED_SFIELD(sfSecureUI,                 VL,        96)
 TYPED_SFIELD(sfHookName,                 VL,        97)
 TYPED_SFIELD(sfRemarkValue,              VL,        98)
 TYPED_SFIELD(sfRemarkName,               VL,        99)

include/xrpl/protocol/detail/transactions.macro

@@ -74,7 +74,6 @@ TRANSACTION(ttACCOUNT_SET, 3, AccountSet, ({
     {sfTickSize, soeOPTIONAL},
     {sfNFTokenMinter, soeOPTIONAL},
     {sfHookStateScale, soeOPTIONAL},
-    {sfSecureUI, soeOPTIONAL},
 }))
 
 /** This transaction type cancels an existing escrow. */

src/test/app/HookAPI_test.cpp

@@ -3632,9 +3632,8 @@ public:
 
         auto const alice = Account{"alice"};
         auto const bob = Account{"bob"};
-        auto const claire = Account{"claire"};
         Env env{*this, features};
-        env.fund(XRP(100000), alice, bob, claire);
+        env.fund(XRP(100000), alice, bob);
         env.close();
 
         // Compute hook hash for the accept hook
@@ -3774,84 +3773,6 @@ public:
             BEAST_EXPECT(result2.has_value());
             BEAST_EXPECT(result2.value() == newData.size());
         }
-
-        {
-            // fixHookMap: foreign state set without grant after state previous
-            // modified
-
-            HookStateMap stateMap;
-            auto hookCtx = makeStubHookContext(
-                applyCtx, alice.id(), bob.id(), {}, stateMap);
-
-            AccountID const aliceid = alice.id();
-
-            // Pre-populate stateMap
-            stateMap[alice.id()] = {
-                100,  // availableForReserves
-                1,    // namespaceCount
-                1,    // hookStateScale
-                {}};
-
-            auto& api = hookCtx.api();
-
-            // setup a hook on alice, and on claire, no grants
-            env(hook(alice, {{hso(genesis::AcceptHook)}}, 0), fee(XRP(1)));
-            env(hook(claire, {{hso(genesis::AcceptHook)}}, 0), fee(XRP(1)));
-            env.close();
-
-            // First modification
-            auto result1 =
-                api.state_foreign_set(testKey, testNs, aliceid, testData);
-            BEAST_EXPECT(result1.has_value());
-
-            // Second modification this time using bob as hookacc (should hit
-            // cache)
-            auto hookCtx2 = makeStubHookContext(
-                applyCtx, claire.id(), bob.id(), {}, stateMap);
-
-            // check the state entry is carried into the second context
-
-            // does the map contain the account?
-            BEAST_EXPECT(
-                hookCtx2.result.stateMap.find(aliceid) !=
-                hookCtx2.result.stateMap.end());
-
-            // the name space?
-            BEAST_EXPECT(
-                std::get<3>(hookCtx2.result.stateMap[aliceid]).find(testNs) !=
-                std::get<3>(hookCtx2.result.stateMap[aliceid]).end());
-
-            // the key entry?
-            BEAST_EXPECT(
-                std::get<3>(hookCtx2.result.stateMap[aliceid])[testNs].find(
-                    testKey) !=
-                std::get<3>(hookCtx2.result.stateMap[aliceid])[testNs].end());
-
-            // is the entry marked as modified?
-            BEAST_EXPECT(
-                std::get<3>(hookCtx2.result.stateMap[aliceid])[testNs][testKey]
-                    .first);
-
-            auto& api2 = hookCtx2.api();
-            Bytes newData{0x04, 0x05};
-            auto result2 =
-                api2.state_foreign_set(testKey, testNs, aliceid, newData);
-
-            if (features[fixHookMap])
-            {
-                // new behaviour: grant is missing, cannot write
-                BEAST_EXPECT(!result2.has_value());
-                BEAST_EXPECT(result2.error() == NOT_AUTHORIZED);
-                BEAST_EXPECT(hookCtx2.result.foreignStateSetDisabled);
-            }
-            else
-            {
-                // old behaviour: allow this illegal write due to the entry
-                // being modified previously in the map
-                BEAST_EXPECT(result2.has_value());
-                BEAST_EXPECT(result2.value() == newData.size());
-            }
-        }
     }
 
     void
@@ -4911,7 +4832,6 @@ public:
 
         test_state(features);
         test_state_foreign(features);
-        test_state_foreign_set(features - fixHookMap);
         test_state_foreign_set(features);
         test_state_foreign_set_max(features);
         test_state_set(features);

src/xrpld/app/hook/applyHook.h

@@ -15,7 +15,6 @@
 #include <memory>
 #include <optional>
 #include <queue>
-#include <utility>
 #include <vector>
 #include <wasmedge/wasmedge.h>
 
@@ -175,8 +174,6 @@ struct HookResult
         false;  // hook_again allows strong pre-apply to nominate
                 // additional weak post-apply execution
     std::shared_ptr<STObject const> provisionalMeta;
-    std::set<std::pair<AccountID, uint256 /* namespace */>>
-        foreignStateGrantCache;  // add found grants here to avoid rechecking
 };
 
 class HookExecutor;

src/xrpld/app/hook/detail/HookAPI.cpp

@@ -1920,114 +1920,88 @@ HookAPI::state_foreign_set(
     if (hookCtx.result.foreignStateSetDisabled)
         return Unexpected(PREVIOUS_FAILURE_PREVENTS_RETRY);
 
-    bool const hasFix = hookCtx.applyCtx.view().rules().enabled(fixHookMap);
-
-    if (!hasFix)
+    // first check if we've already modified this state
+    auto cacheEntry = lookup_state_cache(account, ns, key);
+    if (cacheEntry && cacheEntry->get().first)
     {
-        // first check if we've already modified this state
-        auto cacheEntry = lookup_state_cache(account, ns, key);
-        if (cacheEntry && cacheEntry->get().first)
-        {
-            // if a cache entry already exists and it has already been modified
-            // don't check grants again
-            if (auto ret = set_state_cache(account, ns, key, data, true);
-                !ret.has_value())
-                return Unexpected(ret.error());
+        // if a cache entry already exists and it has already been modified
+        // don't check grants again
+        if (auto ret = set_state_cache(account, ns, key, data, true);
+            !ret.has_value())
+            return Unexpected(ret.error());
 
-            return data.size();
-        }
+        return data.size();
     }
-    // check if we've used a grant to modify this state entry before, if not
-    // look up possible grants
-    if (!hasFix ||
-        hookCtx.result.foreignStateGrantCache.find({account, ns}) ==
-            hookCtx.result.foreignStateGrantCache.end())
+
+    // cache miss or cache was present but entry was not marked as previously
+    // modified therefore before continuing we need to check grants
+    auto const sle =
+        hookCtx.applyCtx.view().read(ripple::keylet::hook(account));
+    if (!sle)
+        return Unexpected(INTERNAL_ERROR);
+
+    bool found_auth = false;
+
+    // we do this by iterating the hooks installed on the foreign account and in
+    // turn their grants and namespaces
+    auto const& hooks = sle->getFieldArray(sfHooks);
+    for (auto const& hookObj : hooks)
     {
-        auto const sle =
-            hookCtx.applyCtx.view().read(ripple::keylet::hook(account));
+        // skip blank entries
+        if (!hookObj.isFieldPresent(sfHookHash))
+            continue;
 
-        if (!sle)
+        if (!hookObj.isFieldPresent(sfHookGrants))
+            continue;
+
+        auto const& hookGrants = hookObj.getFieldArray(sfHookGrants);
+
+        if (hookGrants.size() < 1)
+            continue;
+
+        // the grant allows the hook to modify the granter's namespace only
+        if (hookObj.isFieldPresent(sfHookNamespace))
         {
-            if (hasFix)
-            {
-                hookCtx.result.foreignStateSetDisabled = true;
-                return Unexpected(NOT_AUTHORIZED);
-            }
-
-            return Unexpected(INTERNAL_ERROR);
+            if (hookObj.getFieldH256(sfHookNamespace) != ns)
+                continue;
+        }
+        else
+        {
+            // fetch the hook definition
+            auto const def =
+                hookCtx.applyCtx.view().read(ripple::keylet::hookDefinition(
+                    hookObj.getFieldH256(sfHookHash)));
+            if (!def)  // should never happen except in a rare race condition
+                continue;
+            if (def->getFieldH256(sfHookNamespace) != ns)
+                continue;
         }
 
-        // RH TODO: test this code path more completely
-
-        bool found_auth = false;
-
-        // we do this by iterating the hooks installed on the foreign account
-        // and in turn their grants and namespaces
-        auto const& hooks = sle->getFieldArray(sfHooks);
-        for (auto const& hookObj : hooks)
+        // this is expensive search so we'll disallow after one failed attempt
+        for (auto const& hookGrantObj : hookGrants)
         {
-            // skip blank entries
-            if (!hookObj.isFieldPresent(sfHookHash))
-                continue;
+            bool hasAuthorizedField = hookGrantObj.isFieldPresent(sfAuthorize);
 
-            if (!hookObj.isFieldPresent(sfHookGrants))
-                continue;
-
-            auto const& hookGrants = hookObj.getFieldArray(sfHookGrants);
-
-            if (hookGrants.size() < 1)
-                continue;
-
-            // the grant allows the hook to modify the granter's namespace only
-            if (hookObj.isFieldPresent(sfHookNamespace))
+            if (hookGrantObj.getFieldH256(sfHookHash) ==
+                    hookCtx.result.hookHash &&
+                (!hasAuthorizedField ||
+                 hookGrantObj.getAccountID(sfAuthorize) ==
+                     hookCtx.result.account))
             {
-                if (hookObj.getFieldH256(sfHookNamespace) != ns)
-                    continue;
-            }
-            else
-            {
-                // fetch the hook definition
-                auto const def =
-                    hookCtx.applyCtx.view().read(ripple::keylet::hookDefinition(
-                        hookObj.getFieldH256(sfHookHash)));
-                if (!def)  // should never happen except in a rare race
-                           // condition
-                    continue;
-                if (def->getFieldH256(sfHookNamespace) != ns)
-                    continue;
-            }
-
-            // this is expensive search so we'll disallow after one failed
-            // attempt
-            for (auto const& hookGrantObj : hookGrants)
-            {
-                bool hasAuthorizedField =
-                    hookGrantObj.isFieldPresent(sfAuthorize);
-
-                if (hookGrantObj.getFieldH256(sfHookHash) ==
-                        hookCtx.result.hookHash &&
-                    (!hasAuthorizedField ||
-                     hookGrantObj.getAccountID(sfAuthorize) ==
-                         hookCtx.result.account))
-                {
-                    found_auth = true;
-                    break;
-                }
-            }
-
-            if (found_auth)
+                found_auth = true;
                 break;
+            }
         }
 
-        if (!found_auth)
-        {
-            // hook only gets one attempt
-            hookCtx.result.foreignStateSetDisabled = true;
-            return Unexpected(NOT_AUTHORIZED);
-        }
+        if (found_auth)
+            break;
+    }
 
-        // add the grant to the cache
-        hookCtx.result.foreignStateGrantCache.emplace(account, ns);
+    if (!found_auth)
+    {
+        // hook only gets one attempt
+        hookCtx.result.foreignStateSetDisabled = true;
+        return Unexpected(NOT_AUTHORIZED);
     }
 
     if (auto ret = set_state_cache(account, ns, key, data, true);

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

@@ -197,31 +197,6 @@ SetAccount::preflight(PreflightContext const& ctx)
             return temMALFORMED;
     }
 
-    if (tx.isFieldPresent(sfSecureUI))
-    {
-        if (!ctx.rules.enabled(featureSecureUI))
-            return temMALFORMED;
-
-        Blob ui = tx.getFieldVL(sfSecureUI);
-
-        if (ui.size() == 0)
-        {
-            // this is an unset operation, pass
-        }
-        else if (ui.size() > 4096)
-        {
-            JLOG(j.trace()) << "SecureUI: Too long > 4096 bytes";
-            return temMALFORMED;
-        }
-        else if (!URIToken::validateUTF8(ui))
-        {
-            JLOG(j.trace()) << "SecureUI: Not UTF-8";
-            return temMALFORMED;
-        }
-
-        // valid
-    } 
-
     return preflight2(ctx);
 }
 
@@ -724,23 +699,6 @@ SetAccount::doApply()
             sle->setFieldU16(sfHookStateScale, newScale);
         }
     }
-
-    if (tx.isFieldPresent(sfSecureUI))
-    {
-        Blob ui = tx.getFieldVL(sfSecureUI);
-        if (ui.size() == 0)
-        {
-            // unset operation
-            if (sle->isFieldPresent(sfSecureUI))
-                sle->makeFieldAbsent(sfSecureUI);
-        }
-        else
-        {
-            // set operation
-            sle->setFieldVL(sfSecureUI, std::move(ui));
-        }
-    }
-
     ctx_.view().update(sle);
 
     return tesSUCCESS;