Skip to content

Commit

Permalink
[FOLD] Set owner count based on series size and other changes
Browse files Browse the repository at this point in the history 
* Add unit-tests for owner count
* Change temARRAY_SIZE to temBAD_ARRAY_SIZE
  • Loading branch information
@gregtatcam
gregtatcam committed Nov 14, 2023
1 parent 4faf02b commit 866ed50
Show file tree
Hide file tree
Showing 6 changed files with 142 additions and 67 deletions.
20 changes: 9 additions & 11 deletions src/ripple/app/tx/impl/DeleteOracle.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,16 +69,13 @@ DeleteOracle::preclaim(PreclaimContext const& ctx)
TER
DeleteOracle::doApply()
{
// This is the ledger view that we work against. Transactions are applied
// as we go on processing transactions.
Sandbox sb(&ctx_.view());

if (auto sle = sb.peek(keylet::oracle(account_, ctx_.tx[sfOracleSequence]));
if (auto sle = ctx_.view().peek(
keylet::oracle(account_, ctx_.tx[sfOracleSequence]));
!sle)
return tecINTERNAL;
else
{
if (!sb.dirRemove(
if (!ctx_.view().dirRemove(
keylet::ownerDir(account_),
(*sle)[sfOwnerNode],
sle->key(),
Expand All @@ -88,15 +85,16 @@ DeleteOracle::doApply()
return tefBAD_LEDGER;
}

auto const sleOwner = sb.peek(keylet::account(account_));
auto const sleOwner = ctx_.view().peek(keylet::account(account_));
if (!sleOwner)
return tecINTERNAL;

adjustOwnerCount(sb, sleOwner, -1, j_);
sb.update(sleOwner);
auto const count =
sle->getFieldArray(sfPriceDataSeries).size() > 5 ? -2 : -1;

adjustOwnerCount(ctx_.view(), sleOwner, count, j_);

sb.erase(sle);
sb.apply(ctx_.rawView());
ctx_.view().erase(sle);
}

return tesSUCCESS;
Expand Down
72 changes: 35 additions & 37 deletions src/ripple/app/tx/impl/SetOracle.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ SetOracle::preflight(PreflightContext const& ctx)

auto const& dataSeries = ctx.tx.getFieldArray(sfPriceDataSeries);
if (dataSeries.size() == 0 || dataSeries.size() > maxOracleDataSeries)
return temARRAY_SIZE;
return temBAD_ARRAY_SIZE;

auto invalidLength = [&](auto const& sField, std::size_t length) {
return ctx.tx.isFieldPresent(sField) &&
Expand All @@ -70,7 +70,7 @@ SetOracle::preclaim(PreclaimContext const& ctx)
auto const sleSetter =
ctx.view.read(keylet::account(ctx.tx.getAccountID(sfAccount)));
if (!sleSetter)
return {terNO_ACCOUNT};
return terNO_ACCOUNT;

// lastUpdateTime must be within 30 seconds of the last closed ledger
using namespace std::chrono;
Expand Down Expand Up @@ -126,9 +126,9 @@ SetOracle::preclaim(PreclaimContext const& ctx)
}

if (pairs.size() > maxOracleDataSeries)
return temARRAY_SIZE;
return temBAD_ARRAY_SIZE;

auto const add = pairs.size() <= 5 ? 1 : 2;
auto const add = pairs.size() > 5 ? 2 : 1;
auto const reserve = ctx.view.fees().accountReserve(
sleSetter->getFieldU32(sfOwnerCount) + add);
auto const& balance = sleSetter->getFieldAmount(sfBalance);
Expand All @@ -139,16 +139,25 @@ SetOracle::preclaim(PreclaimContext const& ctx)
return tesSUCCESS;
}

static std::pair<TER, bool>
applySet(
ApplyContext& ctx_,
Sandbox& sb,
AccountID const& account_,
beast::Journal j_)
static bool
adjustOwnerCount(ApplyContext& ctx, std::uint16_t count)
{
if (auto const sleAccount =
ctx.view().peek(keylet::account(ctx.tx[sfAccount])))
{
adjustOwnerCount(ctx.view(), sleAccount, count, ctx.journal);
return true;
}

return false;
}

TER
SetOracle::doApply()
{
auto const oracleID = keylet::oracle(account_, ctx_.tx[sfOracleSequence]);

if (auto sle = sb.peek(oracleID))
if (auto sle = ctx_.view().peek(oracleID))
{
// update
// the token pair that doesn't have their price updated will not
Expand All @@ -165,6 +174,7 @@ applySet(
sfPriceUnit, entry.getFieldCurrency(sfPriceUnit));
pairs.emplace(tokenPairHash(entry), std::move(priceData));
}
auto const oldCount = pairs.size() > 5 ? 2 : 1;
// update/add pairs
for (auto const& entry : ctx_.tx.getFieldArray(sfPriceDataSeries))
{
Expand Down Expand Up @@ -200,53 +210,41 @@ applySet(
sle->setFieldVL(sfURI, ctx_.tx[sfURI]);
sle->setFieldU32(sfLastUpdateTime, ctx_.tx[sfLastUpdateTime]);

sb.update(sle);
auto const newCount = pairs.size() > 5 ? 2 : 1;
if (newCount > oldCount && !adjustOwnerCount(ctx_, 1))
return tefINTERNAL;

ctx_.view().update(sle);
}
else
{
// create

auto const sleAccount = sb.peek(keylet::account(account_));
if (!sleAccount)
return {tefINTERNAL, false};

sle = std::make_shared<SLE>(oracleID);
sle->setAccountID(sfOwner, ctx_.tx.getAccountID(sfAccount));
sle->setFieldVL(sfProvider, ctx_.tx[sfProvider]);
if (ctx_.tx.isFieldPresent(sfURI))
sle->setFieldVL(sfURI, ctx_.tx[sfURI]);
sle->setFieldArray(
sfPriceDataSeries, ctx_.tx.getFieldArray(sfPriceDataSeries));
auto const& series = ctx_.tx.getFieldArray(sfPriceDataSeries);
sle->setFieldArray(sfPriceDataSeries, series);
sle->setFieldVL(sfSymbolClass, ctx_.tx[sfSymbolClass]);
sle->setFieldU32(sfLastUpdateTime, ctx_.tx[sfLastUpdateTime]);

auto page = sb.dirInsert(
auto page = ctx_.view().dirInsert(
keylet::ownerDir(account_), sle->key(), describeOwnerDir(account_));
if (!page)
return {tecDIR_FULL, false};
return tecDIR_FULL;

(*sle)[sfOwnerNode] = *page;

adjustOwnerCount(sb, sleAccount, 1, j_);
auto const count = series.size() > 5 ? 2 : 1;
if (!adjustOwnerCount(ctx_, count))
return tefINTERNAL;

sb.insert(sle);
ctx_.view().insert(sle);
}

return {tesSUCCESS, true};
}

TER
SetOracle::doApply()
{
// This is the ledger view that we work against. Transactions are applied
// as we go on processing transactions.
Sandbox sb(&ctx_.view());

auto const result = applySet(ctx_, sb, account_, j_);
if (result.second)
sb.apply(ctx_.rawView());

return result.first;
return tesSUCCESS;
}

} // namespace ripple
2 changes: 1 addition & 1 deletion src/ripple/protocol/TER.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ enum TEMcodes : TERUnderlyingType {

temEMPTY_DID,

temARRAY_SIZE,
temBAD_ARRAY_SIZE,
};

//------------------------------------------------------------------------------
Expand Down
2 changes: 1 addition & 1 deletion src/ripple/protocol/impl/TER.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ transResults()
MAKE_ERROR(temXCHAIN_BRIDGE_NONDOOR_OWNER, "Malformed: Bridge owner must be one of the door accounts."),
MAKE_ERROR(temXCHAIN_BRIDGE_BAD_MIN_ACCOUNT_CREATE_AMOUNT, "Malformed: Bad min account create amount."),
MAKE_ERROR(temXCHAIN_BRIDGE_BAD_REWARD_AMOUNT, "Malformed: Bad reward amount."),
MAKE_ERROR(temARRAY_SIZE, "Malformed: Invalid array size."),
MAKE_ERROR(temBAD_ARRAY_SIZE, "Malformed: Invalid array size."),

MAKE_ERROR(terRETRY, "Retry transaction."),
MAKE_ERROR(terFUNDS_SPENT, "DEPRECATED."),
Expand Down
100 changes: 85 additions & 15 deletions src/test/app/Oracle_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,20 @@ namespace test {
struct Oracle_test : public beast::unit_test::suite
{
private:
// Helper function that returns the owner count of an account root.
static std::uint32_t
ownerCount(jtx::Env const& env, jtx::Account const& acct)
{
std::uint32_t ret{0};
if (auto const sleAcct = env.le(acct))
ret = sleAcct->at(sfOwnerCount);
return ret;
}

void
testInvalidSet()
{
testcase("Invalid Create");
testcase("Invalid Set");

using namespace jtx;
Account const owner("owner");
Expand Down Expand Up @@ -105,8 +115,8 @@ struct Oracle_test : public beast::unit_test::suite
{"XRP", "US9", 740, 1},
{"XRP", "U10", 750, 1},
{"XRP", "U11", 740, 1}},
.ter = ter(temARRAY_SIZE)});
oracle.set(CreateArg{.series = {}, .ter = ter(temARRAY_SIZE)});
.ter = ter(temBAD_ARRAY_SIZE)});
oracle.set(CreateArg{.series = {}, .ter = ter(temBAD_ARRAY_SIZE)});
}

// Array of token pair exceeds 10 after update
Expand All @@ -132,7 +142,7 @@ struct Oracle_test : public beast::unit_test::suite
{"XRP", "US9", 740, 1},
{"XRP", "U10", 750, 1},
},
.ter = ter(temARRAY_SIZE)});
.ter = ter(temBAD_ARRAY_SIZE)});
}

{
Expand Down Expand Up @@ -230,11 +240,32 @@ struct Oracle_test : public beast::unit_test::suite
using namespace jtx;
Account const owner("owner");

{
Env env(*this);
auto test = [&](Env& env, DataSeries const& series, std::uint16_t adj) {
env.fund(XRP(1'000), owner);
Oracle oracle(env, {.owner = owner});
auto const count = ownerCount(env, owner);
Oracle oracle(env, {.owner = owner, .series = series});
BEAST_EXPECT(oracle.exists());
BEAST_EXPECT(ownerCount(env, owner) == (count + adj));
};

{
// owner count is adjusted by 1
Env env(*this);
test(env, {{"XRP", "USD", 740, 1}}, 1);
}

{
// owner count is adjusted by 2
Env env(*this);
test(
env,
{{"XRP", "USD", 740, 1},
{"BTC", "USD", 740, 1},
{"ETH", "USD", 740, 1},
{"CAN", "USD", 740, 1},
{"YAN", "USD", 740, 1},
{"GBP", "USD", 740, 1}},
2);
}

{
Expand All @@ -245,7 +276,8 @@ struct Oracle_test : public beast::unit_test::suite
env.fund(XRP(1'000), some);
Oracle oracle(env, {.owner = owner});
BEAST_EXPECT(oracle.exists());
oracle.set(CreateArg{.owner = some});
oracle.set(CreateArg{
.owner = some, .series = {{"912810RR9", "USD", 740, 1}}});
BEAST_EXPECT(Oracle::exists(env, some, oracle.sequence()));
}
}
Expand Down Expand Up @@ -284,14 +316,39 @@ struct Oracle_test : public beast::unit_test::suite
{
testcase("Delete");
using namespace jtx;

Env env(*this);
Account const owner("owner");
env.fund(XRP(1'000), owner);
Oracle oracle(env, {.owner = owner});
BEAST_EXPECT(oracle.exists());
oracle.remove({});
BEAST_EXPECT(!oracle.exists());

auto test = [&](Env& env, DataSeries const& series, std::uint16_t adj) {
env.fund(XRP(1'000), owner);
Oracle oracle(env, {.owner = owner, .series = series});
auto const count = ownerCount(env, owner);
BEAST_EXPECT(oracle.exists());
oracle.remove({});
BEAST_EXPECT(!oracle.exists());
BEAST_EXPECT(ownerCount(env, owner) == (count - adj));
};

{
// owner count is adjusted by 1
Env env(*this);
test(env, {{"XRP", "USD", 740, 1}}, 1);
}

{
// owner count is adjusted by 2
Env env(*this);
test(
env,
{
{"XRP", "USD", 740, 1},
{"BTC", "USD", 740, 1},
{"ETH", "USD", 740, 1},
{"CAN", "USD", 740, 1},
{"YAN", "USD", 740, 1},
{"GBP", "USD", 740, 1},
},
2);
}
}

void
Expand All @@ -303,12 +360,15 @@ struct Oracle_test : public beast::unit_test::suite

Env env(*this);
env.fund(XRP(1'000), owner);
auto const count = ownerCount(env, owner);
Oracle oracle(env, {.owner = owner});
BEAST_EXPECT(oracle.exists());

// update existing pair
oracle.set(UpdateArg{.series = {{"XRP", "USD", 740, 2}}});
BEAST_EXPECT(oracle.expectPrice({{"XRP", "USD", 740, 2}}));
// owner count is adjusted by 1
BEAST_EXPECT(ownerCount(env, owner) == (count + 1));

// add new pairs, not-included pair is reset
oracle.set(UpdateArg{.series = {{"XRP", "EUR", 700, 2}}});
Expand All @@ -320,6 +380,16 @@ struct Oracle_test : public beast::unit_test::suite
.series = {{"XRP", "USD", 741, 2}, {"XRP", "EUR", 710, 2}}});
BEAST_EXPECT(oracle.expectPrice(
{{"XRP", "USD", 741, 2}, {"XRP", "EUR", 710, 2}}));

// owner count is adjusted by 2
oracle.set(UpdateArg{
.series = {
{"BTC", "USD", 741, 2},
{"ETH", "EUR", 710, 2},
{"YAN", "EUR", 710, 2},
{"CAN", "EUR", 710, 2},
}});
BEAST_EXPECT(ownerCount(env, owner) == (count + 2));
}

void
Expand Down
13 changes: 11 additions & 2 deletions src/test/jtx/impl/Oracle.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -204,12 +204,21 @@ Oracle::set(UpdateArg const& arg)
.count() +
10);
Json::Value dataSeries(Json::arrayValue);
auto assetToStr = [](std::string const& s) {
// assume standard currency
if (s.size() == 3)
return s;
assert(s.size() <= 20);
// anything else must be 160-bit hex string
std::string h = strHex(s);
return strHex(s).append(40 - s.size() * 2, '0');
};
for (auto const& data : arg.series)
{
Json::Value priceData;
Json::Value price;
price[jss::Symbol] = std::get<0>(data);
price[jss::PriceUnit] = std::get<1>(data);
price[jss::Symbol] = assetToStr(std::get<0>(data));
price[jss::PriceUnit] = assetToStr(std::get<1>(data));
if (std::get<2>(data))
price[jss::SymbolPrice] = *std::get<2>(data);
if (std::get<3>(data))
Expand Down

0 comments on commit 866ed50

Please sign in to comment.