Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixInnerObjTemplate: Add STObject constructor to explicitly set inner object template #4906

Merged
merged 12 commits into from
Feb 7, 2024
Merged
23 changes: 21 additions & 2 deletions src/ripple/app/misc/impl/AMMUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,9 @@ std::uint16_t
getTradingFee(ReadView const& view, SLE const& ammSle, AccountID const& account)
{
using namespace std::chrono;
assert(
!view.rules().enabled(fixInnerObjTemplate) ||
ammSle.isFieldPresent(sfAuctionSlot));
if (ammSle.isFieldPresent(sfAuctionSlot))
{
auto const& auctionSlot =
Expand Down Expand Up @@ -298,7 +301,24 @@ initializeFeeAuctionVote(
voteSlots.push_back(voteEntry);
ammSle->setFieldArray(sfVoteSlots, voteSlots);
// AMM creator gets the auction slot for free.
STObject auctionSlot = STObject::makeInnerObject(sfAuctionSlot, rules);
// AuctionSlot is created on AMMCreate and updated on AMMDeposit
// when AMM is in an empty state
STObject& auctionSlot = [&]() -> STObject& {
if (!rules.enabled(fixInnerObjTemplate))
{
return ammSle->peekFieldObject(sfAuctionSlot);
}
else
{
if (!ammSle->isFieldPresent(sfAuctionSlot))
{
STObject auctionSlot =
STObject::makeInnerObject(sfAuctionSlot, rules);
ammSle->set(std::move(auctionSlot));
}
return ammSle->peekFieldObject(sfAuctionSlot);
}
}();
auctionSlot.setAccountID(sfAccount, account);
// current + sec in 24h
auto const expiration = std::chrono::duration_cast<std::chrono::seconds>(
Expand All @@ -316,7 +336,6 @@ initializeFeeAuctionVote(
auctionSlot.setFieldU16(sfDiscountedFee, dfee);
else if (auctionSlot.isFieldPresent(sfDiscountedFee))
auctionSlot.makeFieldAbsent(sfDiscountedFee);
ammSle->set(&auctionSlot);
}

} // namespace ripple
14 changes: 12 additions & 2 deletions src/ripple/app/tx/impl/AMMBid.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -173,8 +173,18 @@ applyBid(
return {tecINTERNAL, false};
STAmount const lptAMMBalance = (*ammSle)[sfLPTokenBalance];
auto const lpTokens = ammLPHolds(sb, *ammSle, account_, ctx_.journal);
if (!ammSle->isFieldPresent(sfAuctionSlot))
ammSle->makeFieldPresent(sfAuctionSlot);
auto const& rules = ctx_.view().rules();
if (!rules.enabled(fixInnerObjTemplate))
{
if (!ammSle->isFieldPresent(sfAuctionSlot))
ammSle->makeFieldPresent(sfAuctionSlot);
}
else
{
assert(ammSle->isFieldPresent(sfAuctionSlot));
if (!ammSle->isFieldPresent(sfAuctionSlot))
return {tecINTERNAL, false};
}
auto& auctionSlot = ammSle->peekFieldObject(sfAuctionSlot);
auto const current =
duration_cast<seconds>(
Expand Down
4 changes: 4 additions & 0 deletions src/ripple/app/tx/impl/AMMVote.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,10 @@ applyVote(
}
}

assert(
!ctx_.view().rules().enabled(fixInnerObjTemplate) ||
ammSle->isFieldPresent(sfAuctionSlot));

// Update the vote entries and the trading/discounted fee.
ammSle->setFieldArray(sfVoteSlots, updatedVoteSlots);
if (auto const fee = static_cast<std::int64_t>(num / den))
Expand Down
3 changes: 2 additions & 1 deletion src/ripple/protocol/Feature.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ namespace detail {
// Feature.cpp. Because it's only used to reserve storage, and determine how
// large to make the FeatureBitset, it MAY be larger. It MUST NOT be less than
// the actual number of amendments. A LogicError on startup will verify this.
static constexpr std::size_t numFeatures = 66;
static constexpr std::size_t numFeatures = 67;

/** Amendments that this server supports and the default voting behavior.
Whether they are enabled depends on the Rules defined in the validated
Expand Down Expand Up @@ -352,6 +352,7 @@ extern uint256 const featureXChainBridge;
extern uint256 const fixDisallowIncomingV1;
extern uint256 const featureDID;
extern uint256 const fixFillOrKill;
extern uint256 const fixNFTokenReserve;
extern uint256 const fixInnerObjTemplate;

} // namespace ripple
Expand Down
2 changes: 1 addition & 1 deletion src/ripple/protocol/STObject.h
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ class STObject : public STBase, public CountedObject<STObject>
set(std::unique_ptr<STBase> v);

void
set(STBase* v);
set(STBase&& v);

void
setFieldU8(SField const& field, unsigned char);
Expand Down
1 change: 1 addition & 0 deletions src/ripple/protocol/impl/Feature.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -459,6 +459,7 @@ REGISTER_FEATURE(XChainBridge, Supported::yes, VoteBehavior::De
REGISTER_FIX (fixDisallowIncomingV1, Supported::yes, VoteBehavior::DefaultNo);
REGISTER_FEATURE(DID, Supported::yes, VoteBehavior::DefaultNo);
REGISTER_FIX(fixFillOrKill, Supported::yes, VoteBehavior::DefaultNo);
REGISTER_FIX (fixNFTokenReserve, Supported::yes, VoteBehavior::DefaultNo);
REGISTER_FIX(fixInnerObjTemplate, Supported::yes, VoteBehavior::DefaultNo);

// The following amendments are obsolete, but must remain supported
Expand Down
11 changes: 5 additions & 6 deletions src/ripple/protocol/impl/STObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -645,23 +645,22 @@ STObject::getFieldArray(SField const& field) const
void
STObject::set(std::unique_ptr<STBase> v)
{
set(v.get());
set(std::move(*v.get()));
}

void
STObject::set(STBase* v)
STObject::set(STBase&& v)
{
ximinez marked this conversation as resolved.
Show resolved Hide resolved
assert(v);
auto const i = getFieldIndex(v->getFName());
auto const i = getFieldIndex(v.getFName());
if (i != -1)
{
v_[i] = std::move(*v);
v_[i] = std::move(v);
}
else
{
if (!isFree())
Throw<std::runtime_error>("missing field in templated STObject");
v_.emplace_back(std::move(*v));
v_.emplace_back(std::move(v));
}
}

Expand Down
3 changes: 3 additions & 0 deletions src/ripple/rpc/handlers/AMMInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,9 @@ doAMMInfo(RPC::JsonContext& context)
}
if (voteSlots.size() > 0)
ammResult[jss::vote_slots] = std::move(voteSlots);
assert(
!ledger->rules().enabled(fixInnerObjTemplate) ||
amm->isFieldPresent(sfAuctionSlot));
if (amm->isFieldPresent(sfAuctionSlot))
{
auto const& auctionSlot =
Expand Down
18 changes: 7 additions & 11 deletions src/test/app/AMM_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4792,6 +4792,7 @@ struct AMM_test : public jtx::AMMTest
void
testFixDefaultInnerObj()
{
testcase("Fix Default Inner Object");
using namespace jtx;
FeatureBitset const all{supported_amendments()};

Expand All @@ -4812,22 +4813,17 @@ struct AMM_test : public jtx::AMMTest
USD(10),
{.tfee = tfee, .close = closeLedger});
amm.deposit(alice, USD(10), XRP(10));
amm.vote(
alice,
tfee,
std::nullopt,
std::nullopt,
std::nullopt,
ter(err1));
amm.withdraw(gw, USD(1), std::nullopt, std::nullopt, ter(err2));
amm.vote(VoteArg{.account = alice, .tfee = tfee, .err = ter(err1)});
amm.withdraw(WithdrawArg{
.account = gw, .asset1Out = USD(1), .err = ter(err2)});
// with the amendment disabled and ledger not closed,
// second vote succeeds if the first vote sets the trading fee
// to non-zero; if the first vote sets the trading fee to >0 && <9
// then the second withdraw succeeds if the second vote sets
// the trading fee so that the discounted fee is non-zero
amm.vote(
alice, 20, std::nullopt, std::nullopt, std::nullopt, ter(err3));
amm.withdraw(gw, USD(2), std::nullopt, std::nullopt, ter(err4));
amm.vote(VoteArg{.account = alice, .tfee = 20, .err = ter(err3)});
amm.withdraw(WithdrawArg{
.account = gw, .asset1Out = USD(2), .err = ter(err4)});
};

// ledger is closed after each transaction, vote/withdraw don't fail
Expand Down
61 changes: 61 additions & 0 deletions src/test/jtx/AMM.h
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,55 @@ struct CreateArg
bool close = true;
};

struct DepositArg
{
std::optional<Account> account = std::nullopt;
std::optional<LPToken> tokens = std::nullopt;
std::optional<STAmount> asset1In = std::nullopt;
std::optional<STAmount> asset2In = std::nullopt;
std::optional<STAmount> maxEP = std::nullopt;
std::optional<std::uint32_t> flags = std::nullopt;
std::optional<std::pair<Issue, Issue>> assets = std::nullopt;
std::optional<jtx::seq> seq = std::nullopt;
std::optional<std::uint16_t> tfee = std::nullopt;
std::optional<ter> err = std::nullopt;
};

struct WithdrawArg
{
std::optional<Account> account = std::nullopt;
std::optional<LPToken> tokens = std::nullopt;
std::optional<STAmount> asset1Out = std::nullopt;
std::optional<STAmount> asset2Out = std::nullopt;
std::optional<IOUAmount> maxEP = std::nullopt;
std::optional<std::uint32_t> flags = std::nullopt;
std::optional<std::pair<Issue, Issue>> assets = std::nullopt;
std::optional<jtx::seq> seq = std::nullopt;
std::optional<ter> err = std::nullopt;
};

struct VoteArg
{
std::optional<Account> account = std::nullopt;
std::uint32_t tfee = 0;
std::optional<std::uint32_t> flags = std::nullopt;
std::optional<jtx::seq> seq = std::nullopt;
std::optional<std::pair<Issue, Issue>> assets = std::nullopt;
std::optional<ter> err = std::nullopt;
};

struct BidArg
{
std::optional<Account> account = std::nullopt;
std::optional<std::variant<int, IOUAmount, STAmount>> bidMin = std::nullopt;
std::optional<std::variant<int, IOUAmount, STAmount>> bidMax = std::nullopt;
std::vector<Account> authAccounts = {};
std::optional<std::uint32_t> flags = std::nullopt;
std::optional<jtx::seq> seq = std::nullopt;
std::optional<std::pair<Issue, Issue>> assets = std::nullopt;
std::optional<ter> err = std::nullopt;
};

/** Convenience class to test AMM functionality.
*/
class AMM
Expand Down Expand Up @@ -208,6 +257,9 @@ class AMM
std::optional<std::uint16_t> const& tfee = std::nullopt,
std::optional<ter> const& ter = std::nullopt);

IOUAmount
deposit(DepositArg const& arg);

IOUAmount
withdraw(
std::optional<Account> const& account,
Expand Down Expand Up @@ -250,6 +302,9 @@ class AMM
std::optional<jtx::seq> const& seq,
std::optional<ter> const& ter = std::nullopt);

IOUAmount
withdraw(WithdrawArg const& arg);

void
vote(
std::optional<Account> const& account,
Expand All @@ -259,6 +314,9 @@ class AMM
std::optional<std::pair<Issue, Issue>> const& assets = std::nullopt,
std::optional<ter> const& ter = std::nullopt);

void
vote(VoteArg const& arg);

void
bid(std::optional<Account> const& account,
std::optional<std::variant<int, IOUAmount, STAmount>> const& bidMin =
Expand All @@ -271,6 +329,9 @@ class AMM
std::optional<std::pair<Issue, Issue>> const& assets = std::nullopt,
std::optional<ter> const& ter = std::nullopt);

void
bid(BidArg const& arg);

AccountID const&
ammAccount() const
{
Expand Down
Loading
Loading