Introduce StartTime in CronSet and improve next execution scheduling (#596)

This commit is contained in:
tequ
2025-10-21 13:17:53 +09:00
committed by GitHub
parent b0fcd36bcd
commit 6fa6a96e3a
12 changed files with 182 additions and 61 deletions

View File

@@ -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)

View File

@@ -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

View File

@@ -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;
if (afterTime < currentTime)
uint32_t afterTime = lastStartTime + delay;
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);

View File

@@ -51,47 +51,74 @@ SetCron::preflight(PreflightContext const& ctx)
return temINVALID_FLAG;
}
// DelaySeconds (D), RepeatCount (R)
// DR - Set Cron with Delay and Repeat
// D- - Set Cron (once off) with Delay only (repat implicitly 0)
// -R - Invalid
// DelaySeconds (D), RepeatCount (R), StartTime (S)
// DRS - Set Cron with Delay and Repeat and StartTime
// DR- - Invalid(StartTime is required)
// 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 "
"specified to create a cron.";
JLOG(j.debug())
<< "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
if (hasDelay)
{
auto delay = tx.getFieldU32(sfDelaySeconds);
if (delay > 31536000UL /* 365 days in seconds */)
{
JLOG(j.debug()) << "SetCron: DelaySeconds was too high. (max 365 "
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,16 +182,13 @@ 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));
@@ -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 =
hasRepeat ? tx.getFieldU32(sfRepeatCount) + 1 : 1;
uint32_t const additionalExpectedExecutions = 1 + repeatCount;
auto const additionalFee = baseFee * additionalExpectedExecutions;
if (baseFee + additionalFee < baseFee)

View File

@@ -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;

View File

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

View File

@@ -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);

View File

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

View File

@@ -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
// (only RepeatCount specified)
env(cron::set(alice), cron::repeat(256), ter(temMALFORMED));
// Invalid DelaySeconds and RepeatCount combination with StartTime
env(cron::set(alice),
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
BEAST_EXPECT(true);
// Past StartTime
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 =
env.timeKeeper().now().time_since_epoch().count();
baseTime = baseTime + 100;
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();

View File

@@ -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)),

View File

@@ -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
{

View File

@@ -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
{