mirror of
https://github.com/XRPLF/rippled.git
synced 2026-07-29 01:50:43 +00:00
fix(AMM): prevent orphaned objects, inconsistent ledger state: (#4626)
When an AMM account is deleted, the owner directory entries must be
deleted in order to ensure consistent ledger state.
* When deleting AMM account:
* Clean up AMM owner dir, linking AMM account and AMM object
* Delete trust lines to AMM
* Disallow `CheckCreate` to AMM accounts
* AMM cannot cash a check
* Constrain entries in AuthAccounts array to be accounts
* AuthAccounts is an array of objects for the AMMBid transaction
* SetTrust (TrustSet): Allow on AMM only for LP tokens
* If the destination is an AMM account and the trust line doesn't
exist, then:
* If the asset is not the AMM LP token, then fail the tx with
`tecNO_PERMISSION`
* If the AMM is in empty state, then fail the tx with `tecAMM_EMPTY`
* This disallows trustlines to AMM in empty state
* Add AMMID to AMM root account
* Remove lsfAMM flag and use sfAMMID instead
* Remove owner dir entry for ltAMM
* Add `AMMDelete` transaction type to handle amortized deletion
* Limit number of trust lines to delete on final withdraw + AMMDelete
* Put AMM in empty state when LPTokens is 0 upon final withdraw
* Add `tfTwoAssetIfEmpty` deposit option in AMM empty state
* Fail all AMM transactions in AMM empty state except special deposit
* Add `tecINCOMPLETE` to indicate that not all AMM trust lines are
deleted (i.e. partial deletion)
* This is handled in Transactor similar to deleted offers
* Fail AMMDelete with `tecINTERNAL` if AMM root account is nullptr
* Don't validate for invalid asset pair in AMMDelete
* AMMWithdraw deletes AMM trust lines and AMM account/object only if the
number of trust lines is less than max
* Current `maxDeletableAMMTrustLines` = 512
* Check no directory left after AMM trust lines are deleted
* Enable partial trustline deletion in AMMWithdraw
* Add `tecAMM_NOT_EMPTY` to fail any transaction that expects an AMM in
empty state
* Clawback considerations
* Disallow clawback out of AMM account
* Disallow AMM create if issuer can claw back
This patch applies to the AMM implementation in #4294.
Acknowledgements:
Richard Holland and Nik Bougalis for responsibly disclosing this issue.
Bug Bounties and Responsible Disclosures:
We welcome reviews of the project code and urge researchers to
responsibly disclose any issues they may find.
To report a bug, please send a detailed report to:
bugs@xrpl.org
Signed-off-by: Manoj Doshi <mdoshi@ripple.com>
This commit is contained in:
committed by
Manoj Doshi
parent
5530a0b665
commit
58f7aecb0b
@@ -188,4 +188,122 @@ ammAccountHolds(
|
||||
return STAmount{issue};
|
||||
}
|
||||
|
||||
static TER
|
||||
deleteAMMTrustLines(
|
||||
Sandbox& sb,
|
||||
AccountID const& ammAccountID,
|
||||
std::uint16_t maxTrustlinesToDelete,
|
||||
beast::Journal j)
|
||||
{
|
||||
return cleanupOnAccountDelete(
|
||||
sb,
|
||||
keylet::ownerDir(ammAccountID),
|
||||
[&](LedgerEntryType nodeType,
|
||||
uint256 const&,
|
||||
std::shared_ptr<SLE>& sleItem) -> TER {
|
||||
// Should only have the trustlines
|
||||
if (nodeType != LedgerEntryType::ltRIPPLE_STATE)
|
||||
{
|
||||
JLOG(j.error())
|
||||
<< "deleteAMMTrustLines: deleting non-trustline "
|
||||
<< nodeType;
|
||||
return tecINTERNAL;
|
||||
}
|
||||
|
||||
// Trustlines must have zero balance
|
||||
if (sleItem->getFieldAmount(sfBalance) != beast::zero)
|
||||
{
|
||||
JLOG(j.error())
|
||||
<< "deleteAMMTrustLines: deleting trustline with "
|
||||
"non-zero balance.";
|
||||
return tecINTERNAL;
|
||||
}
|
||||
|
||||
return deleteAMMTrustLine(sb, sleItem, ammAccountID, j);
|
||||
},
|
||||
j,
|
||||
maxTrustlinesToDelete);
|
||||
}
|
||||
|
||||
TER
|
||||
deleteAMMAccount(
|
||||
Sandbox& sb,
|
||||
Issue const& asset,
|
||||
Issue const& asset2,
|
||||
beast::Journal j)
|
||||
{
|
||||
auto ammSle = sb.peek(keylet::amm(asset, asset2));
|
||||
if (!ammSle)
|
||||
{
|
||||
JLOG(j.error()) << "deleteAMMAccount: AMM object does not exist "
|
||||
<< asset << " " << asset2;
|
||||
return tecINTERNAL;
|
||||
}
|
||||
|
||||
auto const ammAccountID = (*ammSle)[sfAccount];
|
||||
auto sleAMMRoot = sb.peek(keylet::account(ammAccountID));
|
||||
if (!sleAMMRoot)
|
||||
{
|
||||
JLOG(j.error()) << "deleteAMMAccount: AMM account does not exist "
|
||||
<< to_string(ammAccountID);
|
||||
return tecINTERNAL;
|
||||
}
|
||||
|
||||
if (auto const ter =
|
||||
deleteAMMTrustLines(sb, ammAccountID, maxDeletableAMMTrustLines, j);
|
||||
ter != tesSUCCESS)
|
||||
return ter;
|
||||
|
||||
auto const ownerDirKeylet = keylet::ownerDir(ammAccountID);
|
||||
if (sb.exists(ownerDirKeylet) && !sb.emptyDirDelete(ownerDirKeylet))
|
||||
{
|
||||
JLOG(j.error()) << "deleteAMMAccount: cannot delete root dir node of "
|
||||
<< toBase58(ammAccountID);
|
||||
return tecINTERNAL;
|
||||
}
|
||||
|
||||
sb.erase(ammSle);
|
||||
sb.erase(sleAMMRoot);
|
||||
|
||||
return tesSUCCESS;
|
||||
}
|
||||
|
||||
void
|
||||
initializeFeeAuctionVote(
|
||||
ApplyView& view,
|
||||
std::shared_ptr<SLE>& ammSle,
|
||||
AccountID const& account,
|
||||
Issue const& lptIssue,
|
||||
std::uint16_t tfee)
|
||||
{
|
||||
// AMM creator gets the voting slot.
|
||||
STArray voteSlots;
|
||||
STObject voteEntry{sfVoteEntry};
|
||||
if (tfee != 0)
|
||||
voteEntry.setFieldU16(sfTradingFee, tfee);
|
||||
voteEntry.setFieldU32(sfVoteWeight, VOTE_WEIGHT_SCALE_FACTOR);
|
||||
voteEntry.setAccountID(sfAccount, account);
|
||||
voteSlots.push_back(voteEntry);
|
||||
ammSle->setFieldArray(sfVoteSlots, voteSlots);
|
||||
// AMM creator gets the auction slot for free.
|
||||
auto& auctionSlot = ammSle->peekFieldObject(sfAuctionSlot);
|
||||
auctionSlot.setAccountID(sfAccount, account);
|
||||
// current + sec in 24h
|
||||
auto const expiration = std::chrono::duration_cast<std::chrono::seconds>(
|
||||
view.info().parentCloseTime.time_since_epoch())
|
||||
.count() +
|
||||
TOTAL_TIME_SLOT_SECS;
|
||||
auctionSlot.setFieldU32(sfExpiration, expiration);
|
||||
auctionSlot.setFieldAmount(sfPrice, STAmount{lptIssue, 0});
|
||||
// Set the fee
|
||||
if (tfee != 0)
|
||||
ammSle->setFieldU16(sfTradingFee, tfee);
|
||||
else if (ammSle->isFieldPresent(sfTradingFee))
|
||||
ammSle->makeFieldAbsent(sfTradingFee);
|
||||
if (auto const dfee = tfee / AUCTION_SLOT_DISCOUNTED_FEE_FRACTION)
|
||||
auctionSlot.setFieldU16(sfDiscountedFee, dfee);
|
||||
else if (auctionSlot.isFieldPresent(sfDiscountedFee))
|
||||
auctionSlot.makeFieldAbsent(sfDiscountedFee);
|
||||
}
|
||||
|
||||
} // namespace ripple
|
||||
|
||||
Reference in New Issue
Block a user