Merge branch 'dev' into flap_fix

Co-authored-by: tequ <git@tequ.dev>

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

@@ -94,21 +94,6 @@ Change::preflight(PreflightContext const& ctx)
                                   "of sfImportVLKey, sfActiveValidator";
             return temMALFORMED;
         }
-
-        // if we do specify import_vl_keys in config then we won't approve keys
-        // that aren't on our list
-        if (ctx.tx.isFieldPresent(sfImportVLKey) &&
-            !ctx.app.config().IMPORT_VL_KEYS.empty())
-        {
-            auto const& inner = const_cast<ripple::STTx&>(ctx.tx)
-                                    .getField(sfImportVLKey)
-                                    .downcast<STObject>();
-            auto const pk = inner.getFieldVL(sfPublicKey);
-            std::string const strPk = strHex(makeSlice(pk));
-            if (ctx.app.config().IMPORT_VL_KEYS.find(strPk) ==
-                ctx.app.config().IMPORT_VL_KEYS.end())
-                return telIMPORT_VL_KEY_NOT_RECOGNISED;
-        }
     }
 
     return tesSUCCESS;
@@ -168,9 +153,42 @@ Change::preclaim(PreclaimContext const& ctx)
             return tesSUCCESS;
         case ttAMENDMENT:
         case ttUNL_MODIFY:
-        case ttUNL_REPORT:
         case ttEMIT_FAILURE:
             return tesSUCCESS;
+        case ttUNL_REPORT: {
+            if (!ctx.tx.isFieldPresent(sfImportVLKey) ||
+                ctx.app.config().IMPORT_VL_KEYS.empty())
+                return tesSUCCESS;
+
+            // if we do specify import_vl_keys in config then we won't approve
+            // keys that aren't on our list and/or aren't in the ledger object
+            auto const& inner = const_cast<ripple::STTx&>(ctx.tx)
+                                    .getField(sfImportVLKey)
+                                    .downcast<STObject>();
+            auto const pkBlob = inner.getFieldVL(sfPublicKey);
+            std::string const strPk = strHex(makeSlice(pkBlob));
+            if (ctx.app.config().IMPORT_VL_KEYS.find(strPk) !=
+                ctx.app.config().IMPORT_VL_KEYS.end())
+                return tesSUCCESS;
+
+            auto const pkType = publicKeyType(makeSlice(pkBlob));
+            if (!pkType)
+                return tefINTERNAL;
+
+            PublicKey const pk(makeSlice(pkBlob));
+
+            // check on ledger
+            if (auto const unlRep = ctx.view.read(keylet::UNLReport());
+                unlRep && unlRep->isFieldPresent(sfImportVLKeys))
+            {
+                auto const& vlKeys = unlRep->getFieldArray(sfImportVLKeys);
+                for (auto const& k : vlKeys)
+                    if (PublicKey(k[sfPublicKey]) == pk)
+                        return tesSUCCESS;
+            }
+
+            return telIMPORT_VL_KEY_NOT_RECOGNISED;
+        }
         default:
             return temUNKNOWN;
     }

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

@@ -51,87 +51,55 @@ SetCron::preflight(PreflightContext const& ctx)
         return temINVALID_FLAG;
     }
 
-    // StartAfter(s) DelaySeconds (D), RepeatCount (R)
-
-    // SDR - Set Cron with After, Delay and Repeat
-    // SD- - Invalid, if repeat count isn't included then only start or delay
-    // S-R - Invalid
-    // S-- - Set Cron with After for a once off execution
-
-    // -DR - Set Cron with Delay and Repeat
-    // -D- - Set Cron (once off) with Delay only (repat implicitly 0)
-    // --R - Invalid
-    // --- - Clear any existing cron (succeeds even if there isn't one) / with
+    // DelaySeconds (D), RepeatCount (R)
+    // DR - Set Cron with Delay and Repeat
+    // D- - Set Cron (once off) with Delay only (repat implicitly 0)
+    // -R - Invalid
+    // -- - Clear any existing cron (succeeds even if there isn't one) / with
     // tfCronUnset flag set
 
-    bool const hasStart = tx.isFieldPresent(sfStartAfter);
     bool const hasDelay = tx.isFieldPresent(sfDelaySeconds);
     bool const hasRepeat = tx.isFieldPresent(sfRepeatCount);
 
-    // unset is a special case, handle first
     if (tx.isFlag(tfCronUnset))
     {
-        if (hasDelay || hasRepeat || hasStart)
+        if (hasDelay || hasRepeat)
         {
             JLOG(j.debug()) << "SetCron: tfCronUnset flag cannot be used with "
                                "DelaySeconds or RepeatCount.";
             return temMALFORMED;
         }
-
-        return preflight2(ctx);
     }
-
-    if (hasStart)
+    else
     {
-        if (hasRepeat && hasDelay)
+        if (!hasDelay)
         {
-            // valid, this is a fully specified cron
-            // fall through to validate other fields
-        }
-        else if (!hasRepeat && !hasDelay)
-        {
-            // valid this is a once off cron
-            // no other fields to validate, done
-            return preflight2(ctx);
-        }
-        else
-        {
-            // invalid, must specify both or neither repeat and delay count with
-            // startafter
-            JLOG(j.debug()) << "SetCron: StartAfter can only be used with "
-                               "either both or neither of "
-                               "DelaySeconds and RepeatCount.";
+            JLOG(j.debug()) << "SetCron: DelaySeconds must be "
+                               "specified to create a cron.";
             return temMALFORMED;
         }
-    }
 
-    if (!hasDelay)
-    {
-        JLOG(j.debug()) << "SetCron: DelaySeconds or StartAfter must be "
-                           "specified to create a cron.";
-        return temMALFORMED;
-    }
-
-    // check delay is not too high
-    auto delay = tx.getFieldU32(sfDelaySeconds);
-    if (delay > 31536000UL /* 365 days in seconds */)
-    {
-        JLOG(j.debug()) << "SetCron: DelaySeconds was too high. (max 365 "
-                           "days in seconds).";
-        return temMALFORMED;
-    }
-
-    // check repeat is not too high
-    if (hasRepeat)
-    {
-        auto recur = tx.getFieldU32(sfRepeatCount);
-        if (recur > 256)
+        // check delay is not too high
+        auto delay = tx.getFieldU32(sfDelaySeconds);
+        if (delay > 31536000UL /* 365 days in seconds */)
         {
-            JLOG(j.debug())
-                << "SetCron: RepeatCount too high. Limit is 256. Issue "
-                   "new SetCron to increase.";
+            JLOG(j.debug()) << "SetCron: DelaySeconds was too high. (max 365 "
+                               "days in seconds).";
             return temMALFORMED;
         }
+
+        // check repeat is not too high
+        if (hasRepeat)
+        {
+            auto recur = tx.getFieldU32(sfRepeatCount);
+            if (recur > 256)
+            {
+                JLOG(j.debug())
+                    << "SetCron: RepeatCount too high. Limit is 256. Issue "
+                       "new SetCron to increase.";
+                return temMALFORMED;
+            }
+        }
     }
 
     return preflight2(ctx);
@@ -140,32 +108,6 @@ SetCron::preflight(PreflightContext const& ctx)
 TER
 SetCron::preclaim(PreclaimContext const& ctx)
 {
-    if (ctx.tx.isFieldPresent(sfStartAfter))
-    {
-        uint32_t currentTime =
-            ctx.view.parentCloseTime().time_since_epoch().count();
-        uint32_t afterTime = ctx.tx.getFieldU32(sfStartAfter);
-
-        if (afterTime <= currentTime)
-        {
-            // we'll pass this as though they meant execute asap, similar to a
-            // delay of 0
-            return tesSUCCESS;
-        }
-
-        uint32_t waitSeconds = afterTime - currentTime;
-
-        if (waitSeconds > afterTime)
-            return tefINTERNAL;
-
-        if (waitSeconds >> 31536000UL /* 365 days in seconds */)
-        {
-            JLOG(ctx.j.debug())
-                << "SetCron: DelaySeconds was too high. (max 365 "
-                   "days in seconds).";
-            return tecSTART_AFTER_TOO_HIGH;
-        }
-    }
     return tesSUCCESS;
 }
 
@@ -181,7 +123,6 @@ SetCron::doApply()
     // ledger.
     uint32_t delay{0};
     uint32_t recur{0};
-    uint32_t after{0};
 
     if (!isDelete)
     {
@@ -196,10 +137,7 @@ SetCron::doApply()
     // do all this sanity checking before we modify the ledger...
     // even for a delete operation this will fall through without incident
 
-    uint32_t afterTime = tx.isFieldPresent(sfStartAfter)
-        ? tx.getFieldU32(sfStartAfter)
-        : currentTime + delay;
-
+    uint32_t afterTime = currentTime + delay;
     if (afterTime < currentTime)
         return tefINTERNAL;
 

src/ripple/protocol/SField.h

@@ -412,7 +412,6 @@ extern SF_UINT32 const sfImportSequence;
 extern SF_UINT32 const sfXahauActivationLgrSeq;
 extern SF_UINT32 const sfDelaySeconds;
 extern SF_UINT32 const sfRepeatCount;
-extern SF_UINT32 const sfStartAfter;
 
 // 64-bit integers (common)
 extern SF_UINT64 const sfIndexNext;

src/ripple/protocol/TER.h

@@ -343,7 +343,6 @@ enum TECcodes : TERUnderlyingType {
     tecINSUF_RESERVE_SELLER = 187,
     tecIMMUTABLE = 188,
     tecTOO_MANY_REMARKS = 189,
-    tecSTART_AFTER_TOO_HIGH = 190,
     tecLAST_POSSIBLE_ENTRY = 255,
 };
 

src/ripple/protocol/impl/SField.cpp

@@ -157,7 +157,6 @@ CONSTRUCT_TYPED_SFIELD(sfLockCount,             "LockCount",            UINT32,
 
 CONSTRUCT_TYPED_SFIELD(sfFirstNFTokenSequence,  "FirstNFTokenSequence", UINT32,    50);
 
-CONSTRUCT_TYPED_SFIELD(sfStartAfter,            "StartAfter",           UINT32,    93);
 CONSTRUCT_TYPED_SFIELD(sfRepeatCount,           "RepeatCount",          UINT32,    94);
 CONSTRUCT_TYPED_SFIELD(sfDelaySeconds,          "DelaySeconds",         UINT32,    95);
 CONSTRUCT_TYPED_SFIELD(sfXahauActivationLgrSeq, "XahauActivationLgrSeq",UINT32,    96);

src/ripple/protocol/impl/TER.cpp

@@ -94,7 +94,6 @@ transResults()
         MAKE_ERROR(tecINSUF_RESERVE_SELLER,          "The seller of an object has insufficient reserves, and thus cannot complete the sale."),
         MAKE_ERROR(tecIMMUTABLE,                     "The remark is marked immutable on the object, and therefore cannot be updated."),
         MAKE_ERROR(tecTOO_MANY_REMARKS,              "The number of remarks on the object would exceed the limit of 32."),
-        MAKE_ERROR(tecSTART_AFTER_TOO_HIGH,          "The proposed StartAfter time is greater than one year away."),
         MAKE_ERROR(tefALREADY,                     "The exact transaction was already in this ledger."),
         MAKE_ERROR(tefBAD_ADD_AUTH,                "Not authorized to add account."),
         MAKE_ERROR(tefBAD_AUTH,                    "Transaction's public key is not authorized."),

src/ripple/protocol/impl/TxFormats.cpp

@@ -486,7 +486,6 @@ TxFormats::TxFormats()
         {
             {sfDelaySeconds, soeOPTIONAL},
             {sfRepeatCount, soeOPTIONAL},
-            {sfStartAfter, soeOPTIONAL},
         },
         commonFields);
 }

src/test/consensus/UNLReport_test.cpp

@@ -357,6 +357,32 @@ class UNLReport_test : public beast::unit_test::suite
             BEAST_EXPECT(isImportVL(env, ivlKeys[0]) == true);
             BEAST_EXPECT(isImportVL(env, ivlKeys[1]) == false);
             BEAST_EXPECT(isActiveValidator(env, vlKeys[0]) == true);
+
+            // now test unrecognised keys that are already present in the ledger
+            // object (flap fix)
+            l = std::make_shared<Ledger>(
+                *l, env.app().timeKeeper().closeTime());
+
+            // insert a ttUNL_REPORT pseudo into the open ledger
+            env.app().openLedger().modify(
+                [&](OpenView& view, beast::Journal j) -> bool {
+                    STTx tx = createUNLRTx(l->seq(), ivlKeys[1], vlKeys[0]);
+                    uint256 txID = tx.getTransactionID();
+                    auto s = std::make_shared<ripple::Serializer>();
+                    tx.add(*s);
+                    env.app().getHashRouter().setFlags(txID, SF_PRIVATE2);
+                    view.rawTxInsert(txID, std::move(s), nullptr);
+                    return true;
+                });
+
+            BEAST_EXPECT(hasUNLReport(env) == true);
+
+            // close the ledger
+            env.close();
+
+            BEAST_EXPECT(isImportVL(env, ivlKeys[0]) == true);
+            BEAST_EXPECT(isImportVL(env, ivlKeys[1]) == false);
+            BEAST_EXPECT(isActiveValidator(env, vlKeys[0]) == true);
         }
     }
 
@@ -1324,4 +1350,4 @@ createUNLRTx(
 }
 
 }  // namespace test
-}  // namespace ripple
+}  // namespace ripple