Merge branch 'dev' into contrib-update

hook/sfcodes.h

@@ -62,6 +62,9 @@
 #define sfEmitGeneration ((2U << 16U) + 46U)
 #define sfLockCount ((2U << 16U) + 49U)
 #define sfFirstNFTokenSequence ((2U << 16U) + 50U)
+#define sfStartTime ((2U << 16U) + 93U)
+#define sfRepeatCount ((2U << 16U) + 94U)
+#define sfDelaySeconds ((2U << 16U) + 95U)
 #define sfXahauActivationLgrSeq ((2U << 16U) + 96U)
 #define sfImportSequence ((2U << 16U) + 97U)
 #define sfRewardTime ((2U << 16U) + 98U)
@@ -129,6 +132,7 @@
 #define sfGovernanceFlags ((5U << 16U) + 99U)
 #define sfGovernanceMarks ((5U << 16U) + 98U)
 #define sfEmittedTxnID ((5U << 16U) + 97U)
+#define sfCron ((5U << 16U) + 95U)
 #define sfAmount ((6U << 16U) + 1U)
 #define sfBalance ((6U << 16U) + 2U)
 #define sfLimitAmount ((6U << 16U) + 3U)

hook/tts.h

@@ -31,6 +31,8 @@
 #define ttURITOKEN_BUY 47
 #define ttURITOKEN_CREATE_SELL_OFFER 48
 #define ttURITOKEN_CANCEL_SELL_OFFER 49
+#define ttCRON 92
+#define ttCRON_SET 93
 #define ttREMIT 95
 #define ttGENESIS_MINT 96
 #define ttIMPORT 97

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

@@ -121,11 +121,11 @@ Cron::doApply()
     uint32_t delay = sleCron->getFieldU32(sfDelaySeconds);
     uint32_t recur = sleCron->getFieldU32(sfRepeatCount);
 
-    uint32_t currentTime = view.parentCloseTime().time_since_epoch().count();
+    uint32_t lastStartTime = sleCron->getFieldU32(sfStartTime);
 
     // do all this sanity checking before we modify the ledger...
-    uint32_t afterTime = currentTime + delay;
+    uint32_t afterTime = lastStartTime + delay;
-    if (afterTime < currentTime)
+    if (afterTime < lastStartTime)
         return tefINTERNAL;
 
     // in all circumstances the Cron object is deleted...
@@ -163,6 +163,7 @@ Cron::doApply()
     sleCron->setFieldU64(sfOwnerNode, *page);
     sleCron->setFieldU32(sfDelaySeconds, delay);
     sleCron->setFieldU32(sfRepeatCount, recur - 1);
+    sleCron->setFieldU32(sfStartTime, afterTime);
     sleCron->setAccountID(sfOwner, id);
 
     sle->setFieldH256(sfCron, klCron.key);

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

@@ -51,47 +51,74 @@ SetCron::preflight(PreflightContext const& ctx)
         return temINVALID_FLAG;
     }
 
-    // DelaySeconds (D), RepeatCount (R)
+    // DelaySeconds (D), RepeatCount (R), StartTime (S)
-    // DR - Set Cron with Delay and Repeat
+    // DRS - Set Cron with Delay and Repeat and StartTime
-    // D- - Set Cron (once off) with Delay only (repat implicitly 0)
+    // DR- - Invalid(StartTime is required)
-    // -R - Invalid
+    // D-S - Invalid (both DelaySeconds and RepeatCount are required)
+    // -RS - Invalid (both DelaySeconds and RepeatCount are required)
+    // --S - Onetime cron with StartTime only
     // -- - Clear any existing cron (succeeds even if there isn't one) / with
     // tfCronUnset flag set
 
     bool const hasDelay = tx.isFieldPresent(sfDelaySeconds);
     bool const hasRepeat = tx.isFieldPresent(sfRepeatCount);
+    bool const hasStartTime = tx.isFieldPresent(sfStartTime);
 
     if (tx.isFlag(tfCronUnset))
     {
-        if (hasDelay || hasRepeat)
+        // delete operation
+        if (hasDelay || hasRepeat || hasStartTime)
         {
             JLOG(j.debug()) << "SetCron: tfCronUnset flag cannot be used with "
-                               "DelaySeconds or RepeatCount.";
+                               "DelaySeconds, RepeatCount or StartTime.";
             return temMALFORMED;
         }
     }
     else
     {
-        if (!hasDelay)
+        // create operation
+
+        if (!hasStartTime)
         {
-            JLOG(j.debug()) << "SetCron: DelaySeconds must be "
+            JLOG(j.debug())
-                               "specified to create a cron.";
+                << "SetCron: StartTime is required. Use StartTime=0 for "
+                   "immediate execution, or specify a future timestamp.";
+            return temMALFORMED;
+        }
+
+        if ((!hasDelay && hasRepeat) || (hasDelay && !hasRepeat))
+        {
+            JLOG(j.debug())
+                << "SetCron: DelaySeconds and RepeatCount must both be present "
+                   "for recurring crons, or both absent for one-off crons.";
             return temMALFORMED;
         }
 
         // check delay is not too high
-        auto delay = tx.getFieldU32(sfDelaySeconds);
+        if (hasDelay)
-        if (delay > 31536000UL /* 365 days in seconds */)
         {
-            JLOG(j.debug()) << "SetCron: DelaySeconds was too high. (max 365 "
+            auto delay = tx.getFieldU32(sfDelaySeconds);
-                               "days in seconds).";
+            if (delay > 31536000UL /* 365 days in seconds */)
-            return temMALFORMED;
+            {
+                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 == 0)
+            {
+                JLOG(j.debug())
+                    << "SetCron: RepeatCount must be greater than 0."
+                       "For one-time execution, omit DelaySeconds and "
+                       "RepeatCount.";
+                return temMALFORMED;
+            }
             if (recur > 256)
             {
                 JLOG(j.debug())
@@ -108,6 +135,30 @@ SetCron::preflight(PreflightContext const& ctx)
 TER
 SetCron::preclaim(PreclaimContext const& ctx)
 {
+    if (ctx.tx.isFieldPresent(sfStartTime) &&
+        ctx.tx.getFieldU32(sfStartTime) != 0)
+    {
+        // StartTime 0 means the cron will execute immediately
+
+        auto const startTime = ctx.tx.getFieldU32(sfStartTime);
+        auto const parentCloseTime =
+            ctx.view.parentCloseTime().time_since_epoch().count();
+
+        if (startTime < parentCloseTime)
+        {
+            JLOG(ctx.j.debug()) << "SetCron: StartTime must be in the future "
+                                   "(or 0 for immediate execution)";
+            return tecEXPIRED;
+        }
+
+        if (startTime > ctx.view.parentCloseTime().time_since_epoch().count() +
+                365 * 24 * 60 * 60)
+        {
+            JLOG(ctx.j.debug()) << "SetCron: StartTime is too far in the "
+                                   "future (max 365 days).";
+            return tecEXPIRED;
+        }
+    }
     return tesSUCCESS;
 }
 
@@ -123,6 +174,7 @@ SetCron::doApply()
     // ledger.
     uint32_t delay{0};
     uint32_t recur{0};
+    uint32_t startTime{0};
 
     if (!isDelete)
     {
@@ -130,17 +182,14 @@ SetCron::doApply()
             delay = tx.getFieldU32(sfDelaySeconds);
         if (tx.isFieldPresent(sfRepeatCount))
             recur = tx.getFieldU32(sfRepeatCount);
+        if (tx.isFieldPresent(sfStartTime))
+        {
+            startTime = tx.getFieldU32(sfStartTime);
+            if (startTime == 0)
+                startTime = view.parentCloseTime().time_since_epoch().count();
+        }
     }
 
-    uint32_t currentTime = view.parentCloseTime().time_since_epoch().count();
-
-    // do all this sanity checking before we modify the ledger...
-    // even for a delete operation this will fall through without incident
-
-    uint32_t afterTime = currentTime + delay;
-    if (afterTime < currentTime)
-        return tefINTERNAL;
-
     AccountID const& id = tx.getAccountID(sfAccount);
     auto sle = view.peek(keylet::account(id));
     if (!sle)
@@ -190,7 +239,7 @@ SetCron::doApply()
     // execution to here means we're creating a new Cron object and adding it to
     // the user's owner dir
 
-    Keylet klCron = keylet::cron(afterTime, id);
+    Keylet klCron = keylet::cron(startTime, id);
 
     std::shared_ptr<SLE> sleCron = std::make_shared<SLE>(klCron);
 
@@ -214,6 +263,7 @@ SetCron::doApply()
     adjustOwnerCount(view, sle, 1, j_);
 
     // set the fields
+    sleCron->setFieldU32(sfStartTime, startTime);
     sleCron->setFieldU32(sfDelaySeconds, delay);
     sleCron->setFieldU32(sfRepeatCount, recur);
     sleCron->setAccountID(sfOwner, id);
@@ -232,18 +282,18 @@ SetCron::calculateBaseFee(ReadView const& view, STTx const& tx)
 {
     auto const baseFee = Transactor::calculateBaseFee(view, tx);
 
-    auto const hasRepeat = tx.isFieldPresent(sfRepeatCount);
-
     if (tx.isFlag(tfCronUnset))
         // delete cron
         return baseFee;
 
+    auto const repeatCount =
+        tx.isFieldPresent(sfRepeatCount) ? tx.getFieldU32(sfRepeatCount) : 0;
+
     // factor a cost based on the total number of txns expected
     // for RepeatCount of 0 we have this txn (SetCron) and the
     // single Cron txn (2). For a RepeatCount of 1 we have this txn,
     // the first time the cron executes, and the second time (3).
-    uint32_t const additionalExpectedExecutions =
+    uint32_t const additionalExpectedExecutions = 1 + repeatCount;
-        hasRepeat ? tx.getFieldU32(sfRepeatCount) + 1 : 1;
     auto const additionalFee = baseFee * additionalExpectedExecutions;
 
     if (baseFee + additionalFee < baseFee)

src/ripple/protocol/SField.h

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

src/ripple/protocol/impl/LedgerFormats.cpp

@@ -371,6 +371,7 @@ LedgerFormats::LedgerFormats()
         ltCRON,
         {
             {sfOwner,                soeREQUIRED},
+            {sfStartTime,            soeREQUIRED},
             {sfDelaySeconds,         soeREQUIRED},
             {sfRepeatCount,          soeREQUIRED},
             {sfOwnerNode,            soeREQUIRED},

src/ripple/protocol/impl/SField.cpp

@@ -157,6 +157,7 @@ CONSTRUCT_TYPED_SFIELD(sfLockCount,             "LockCount",            UINT32,
 
 CONSTRUCT_TYPED_SFIELD(sfFirstNFTokenSequence,  "FirstNFTokenSequence", UINT32,    50);
 
+CONSTRUCT_TYPED_SFIELD(sfStartTime,             "StartTime",            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/TxFormats.cpp

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

src/test/app/Cron_test.cpp

@@ -48,9 +48,13 @@ struct Cron_test : public beast::unit_test::suite
             auto const expectResult =
                 withCron ? ter(tesSUCCESS) : ter(temDISABLED);
 
-            auto tx = cron::set(alice);
             // CLAIM
-            env(cron::set(alice), cron::delay(100), fee(XRP(1)), expectResult);
+            env(cron::set(alice),
+                cron::startTime(0),
+                cron::repeat(100),
+                cron::delay(100),
+                fee(XRP(1)),
+                expectResult);
             env.close();
         }
     }
@@ -70,9 +74,10 @@ struct Cron_test : public beast::unit_test::suite
         env.fund(XRP(1000), alice);
         env.close();
 
-        // create with RepeatCount
+        // create
         auto expected = baseFee * 2 + baseFee * 256;
         env(cron::set(alice),
+            cron::startTime(0),
             cron::delay(356 * 24 * 60 * 60),
             cron::repeat(256),
             fee(expected - 1),
@@ -80,26 +85,13 @@ struct Cron_test : public beast::unit_test::suite
         env.close();
 
         env(cron::set(alice),
+            cron::startTime(0),
             cron::delay(356 * 24 * 60 * 60),
             cron::repeat(256),
             fee(expected),
             ter(tesSUCCESS));
         env.close();
 
-        // create with no RepeatCount
-        expected = baseFee * 2;
-        env(cron::set(alice),
-            cron::delay(356 * 24 * 60 * 60),
-            fee(expected - 1),
-            ter(telINSUF_FEE_P));
-        env.close();
-
-        env(cron::set(alice),
-            cron::delay(356 * 24 * 60 * 60),
-            fee(expected),
-            ter(tesSUCCESS));
-        env.close();
-
         // delete
         expected = baseFee;
         env(cron::set(alice),
@@ -143,30 +135,47 @@ struct Cron_test : public beast::unit_test::suite
 
         // temMALFORMED
         {
-            // Invalid both DelaySeconds and RepeatCount are not specified
+            // Invalid DelaySeconds and RepeatCount and StartTime are not
+            // specified
             env(cron::set(alice), ter(temMALFORMED));
 
-            // Invalid DelaySeconds and RepeatCount combination
+            // Invalid DelaySeconds and RepeatCount combination with StartTime
-            // (only RepeatCount specified)
+            env(cron::set(alice),
-            env(cron::set(alice), cron::repeat(256), ter(temMALFORMED));
+                cron::startTime(100),
+                cron::delay(356 * 24 * 60 * 60),
+                ter(temMALFORMED));
+            env(cron::set(alice),
+                cron::startTime(100),
+                cron::repeat(256),
+                ter(temMALFORMED));
 
             // Invalid DelaySeconds
             env(cron::set(alice),
+                cron::startTime(100),
                 cron::delay(365 * 24 * 60 * 60 + 1),
                 cron::repeat(256),
                 ter(temMALFORMED));
 
             // Invalid RepeatCount
             env(cron::set(alice),
+                cron::startTime(100),
                 cron::delay(365 * 24 * 60 * 60),
                 cron::repeat(257),
                 ter(temMALFORMED));
 
-            // Invalid tfCronUnset flag
+            // Invalid with tfCronUnset flag
             env(cron::set(alice),
                 cron::delay(365 * 24 * 60 * 60),
                 txflags(tfCronUnset),
                 ter(temMALFORMED));
+            env(cron::set(alice),
+                cron::repeat(100),
+                txflags(tfCronUnset),
+                ter(temMALFORMED));
+            env(cron::set(alice),
+                cron::startTime(100),
+                txflags(tfCronUnset),
+                ter(temMALFORMED));
         }
     }
 
@@ -179,9 +188,25 @@ struct Cron_test : public beast::unit_test::suite
 
         auto const alice = Account("alice");
         Env env{*this, features | featureCron};
+        env.fund(XRP(1000), alice);
+        env.close();
 
-        // there is no check in preclaim
+        // Past StartTime
-        BEAST_EXPECT(true);
+        env(cron::set(alice),
+            cron::startTime(
+                env.timeKeeper().now().time_since_epoch().count() - 1),
+            fee(XRP(1)),
+            ter(tecEXPIRED));
+        env.close();
+
+        // Too far Future StartTime
+        env(cron::set(alice),
+            cron::startTime(
+                env.timeKeeper().now().time_since_epoch().count() +
+                365 * 24 * 60 * 60 + 1),
+            fee(XRP(1)),
+            ter(tecEXPIRED));
+        env.close();
     }
 
     void
@@ -199,7 +224,10 @@ struct Cron_test : public beast::unit_test::suite
         auto const aliceOwnerCount = ownerCount(env, alice);
 
         // create cron
+        auto parentCloseTime =
+            env.current()->parentCloseTime().time_since_epoch().count();
         env(cron::set(alice),
+            cron::startTime(parentCloseTime + 356 * 24 * 60 * 60),
             cron::delay(356 * 24 * 60 * 60),
             cron::repeat(256),
             fee(XRP(1)),
@@ -219,9 +247,15 @@ struct Cron_test : public beast::unit_test::suite
         BEAST_EXPECT(
             cronSle->getFieldU32(sfDelaySeconds) == 356 * 24 * 60 * 60);
         BEAST_EXPECT(cronSle->getFieldU32(sfRepeatCount) == 256);
+        BEAST_EXPECT(
+            cronSle->getFieldU32(sfStartTime) ==
+            parentCloseTime + 356 * 24 * 60 * 60);
 
         // update cron
+        parentCloseTime =
+            env.current()->parentCloseTime().time_since_epoch().count();
         env(cron::set(alice),
+            cron::startTime(0),
             cron::delay(100),
             cron::repeat(10),
             fee(XRP(1)),
@@ -243,6 +277,7 @@ struct Cron_test : public beast::unit_test::suite
         BEAST_EXPECT(cronSle2);
         BEAST_EXPECT(cronSle2->getFieldU32(sfDelaySeconds) == 100);
         BEAST_EXPECT(cronSle2->getFieldU32(sfRepeatCount) == 10);
+        BEAST_EXPECT(cronSle2->getFieldU32(sfStartTime) == parentCloseTime);
 
         // delete cron
         env(cron::set(alice),
@@ -289,6 +324,7 @@ struct Cron_test : public beast::unit_test::suite
             auto repeatCount = 10;
 
             env(cron::set(alice),
+                cron::startTime(baseTime + 100),
                 cron::delay(100),
                 cron::repeat(repeatCount),
                 fee(XRP(1)));
@@ -311,7 +347,7 @@ struct Cron_test : public beast::unit_test::suite
                 }
 
                 // close after 100 seconds passed
-                env.close();
+                env.close(10s);
 
                 auto txns = env.closed()->txs;
                 auto size = std::distance(txns.begin(), txns.end());
@@ -348,8 +384,7 @@ struct Cron_test : public beast::unit_test::suite
                             cronSle->getAccountID(sfOwner) == alice.id());
 
                         // set new base time
-                        baseTime =
+                        baseTime = baseTime + 100;
-                            env.timeKeeper().now().time_since_epoch().count();
                         lastCronKeylet = cronKeylet;
                     }
                     else
@@ -380,7 +415,7 @@ struct Cron_test : public beast::unit_test::suite
 
             for (auto const& account : accounts)
             {
-                env(cron::set(account), cron::delay(0), fee(XRP(1)));
+                env(cron::set(account), cron::startTime(0), fee(XRP(1)));
             }
             env.close();
 

src/test/app/SetHookTSH_test.cpp

@@ -6297,6 +6297,7 @@ private:
 
             // cron set
             env(cron::set(account),
+                cron::startTime(0),
                 cron::delay(100),
                 cron::repeat(1),
                 fee(XRP(1)),
@@ -6332,8 +6333,11 @@ private:
             env.fund(XRP(1000), account);
             env.close();
 
+            auto const baseTime =
+                env.current()->parentCloseTime().time_since_epoch().count();
             // cron set
             env(cron::set(account),
+                cron::startTime(baseTime + 100),
                 cron::delay(100),
                 cron::repeat(1),
                 fee(XRP(1)),

src/test/jtx/cron.h

@@ -34,6 +34,21 @@ namespace cron {
 Json::Value
 set(jtx::Account const& account);
 
+/** Sets the optional StartTime on a JTx. */
+class startTime
+{
+private:
+    uint32_t startTime_;
+
+public:
+    explicit startTime(uint32_t startTime) : startTime_(startTime)
+    {
+    }
+
+    void
+    operator()(Env&, JTx& jtx) const;
+};
+
 /** Sets the optional DelaySeconds on a JTx. */
 class delay
 {

src/test/jtx/impl/cron.cpp

@@ -37,6 +37,12 @@ set(jtx::Account const& account)
     return jv;
 }
 
+void
+startTime::operator()(Env& env, JTx& jt) const
+{
+    jt.jv[sfStartTime.jsonName] = startTime_;
+}
+
 void
 delay::operator()(Env& env, JTx& jt) const
 {