Skip to content

Commit

Permalink
Merge pull request #14 from shawnxie999/fix-mpt-delete-bug
Browse files Browse the repository at this point in the history
Fix mpt delete bug
  • Loading branch information
gregtatcam authored Jan 17, 2024
2 parents e7d6e84 + 9e43eb2 commit 4bd78ab
Show file tree
Hide file tree
Showing 5 changed files with 207 additions and 124 deletions.
250 changes: 129 additions & 121 deletions src/ripple/app/tx/impl/MPTokenAuthorize.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,68 +46,81 @@ MPTokenAuthorize::preflight(PreflightContext const& ctx)
TER
MPTokenAuthorize::preclaim(PreclaimContext const& ctx)
{
auto const sleMptIssuance =
ctx.view.read(keylet::mptIssuance(ctx.tx[sfMPTokenIssuanceID]));
if (!sleMptIssuance)
return tecOBJECT_NOT_FOUND;

auto const accountID = ctx.tx[sfAccount];
auto const txFlags = ctx.tx.getFlags();
auto const holderID = ctx.tx[~sfMPTokenHolder];

if (holderID && !(ctx.view.exists(keylet::account(*holderID))))
return tecNO_DST;

std::uint32_t const mptIssuanceFlags = sleMptIssuance->getFieldU32(sfFlags);

// If tx is submitted by issuer, they would either try to do the following
// for allowlisting:
// 1. authorize an account
// 2. unauthorize an account
// if non-issuer account submits this tx, then they are trying either:
// 1. Unauthorize/delete MPToken
// 2. Use/create MPToken
//
// Note: `accountID` is issuer's account
// `holderID` is holder's account
if (accountID == (*sleMptIssuance)[sfIssuer])
// Note: `accountID` is holder's account
// `holderID` is NOT used
if (!holderID)
{
// If tx is submitted by issuer, it only applies for MPT with
// lsfMPTRequireAuth set
if (!(mptIssuanceFlags & lsfMPTRequireAuth))
return tecNO_AUTH;
std::shared_ptr<SLE const> sleMpt = ctx.view.read(
keylet::mptoken(ctx.tx[sfMPTokenIssuanceID], accountID));

if (!holderID)
// There is an edge case where holder deletes MPT after issuance has
// already been destroyed. So we must check for unauthorize before
// fetching the MPTIssuance object(since it doesn't exist)

// if holder wants to delete/unauthorize a mpt
if (ctx.tx.getFlags() & tfMPTUnauthorize)
{
if (!sleMpt)
return tecOBJECT_NOT_FOUND;

if ((*sleMpt)[sfMPTAmount] != 0)
return tecHAS_OBLIGATIONS;

return tesSUCCESS;
}

// Now test when the holder wants to hold/create/authorize a new MPT
auto const sleMptIssuance =
ctx.view.read(keylet::mptIssuance(ctx.tx[sfMPTokenIssuanceID]));

if (!sleMptIssuance)
return tecOBJECT_NOT_FOUND;

if (accountID == (*sleMptIssuance)[sfIssuer])
return temMALFORMED;

if (!ctx.view.exists(
keylet::mptoken(ctx.tx[sfMPTokenIssuanceID], *holderID)))
return tecNO_ENTRY;
// if holder wants to use and create a mpt
if (sleMpt)
return tecMPTOKEN_EXISTS;

return tesSUCCESS;
}

// if non-issuer account submits this tx, then they are trying either:
// 1. Unauthorize/delete MPToken
// 2. Use/create MPToken
auto const sleMptIssuance =
ctx.view.read(keylet::mptIssuance(ctx.tx[sfMPTokenIssuanceID]));
if (!sleMptIssuance)
return tecOBJECT_NOT_FOUND;

std::uint32_t const mptIssuanceFlags = sleMptIssuance->getFieldU32(sfFlags);

// If tx is submitted by issuer, they would either try to do the following
// for allowlisting:
// 1. authorize an account
// 2. unauthorize an account
//
// Note: `accountID` is holder's account
// `holderID` is NOT used
if (holderID)
// Note: `accountID` is issuer's account
// `holderID` is holder's account
if (accountID != (*sleMptIssuance)[sfIssuer])
return temMALFORMED;

std::shared_ptr<SLE const> sleMpt =
ctx.view.read(keylet::mptoken(ctx.tx[sfMPTokenIssuanceID], accountID));

// if holder wants to delete/unauthorize a mpt
if (txFlags & tfMPTUnauthorize)
{
if (!sleMpt)
return tecNO_ENTRY;
// If tx is submitted by issuer, it only applies for MPT with
// lsfMPTRequireAuth set
if (!(mptIssuanceFlags & lsfMPTRequireAuth))
return tecNO_AUTH;

if ((*sleMpt)[sfMPTAmount] != 0)
return tecHAS_OBLIGATIONS;
}
// if holder wants to use and create a mpt
else if (sleMpt)
return tecMPTOKEN_EXISTS;
if (!ctx.view.exists(
keylet::mptoken(ctx.tx[sfMPTokenIssuanceID], *holderID)))
return tecOBJECT_NOT_FOUND;

return tesSUCCESS;
}
Expand All @@ -116,107 +129,102 @@ TER
MPTokenAuthorize::doApply()
{
auto const mptIssuanceID = ctx_.tx[sfMPTokenIssuanceID];
auto const sleMptIssuance = view().read(keylet::mptIssuance(mptIssuanceID));
if (!sleMptIssuance)
return tecINTERNAL;

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

auto const holderID = ctx_.tx[~sfMPTokenHolder];
auto const txFlags = ctx_.tx.getFlags();

// If the account that submitted this tx is the issuer of the MPT
// Note: `account_` is issuer's account
// `holderID` is holder's account
if (account_ == (*sleMptIssuance)[sfIssuer])
{
if (!holderID)
return tecINTERNAL;

auto const sleMpt =
view().peek(keylet::mptoken(mptIssuanceID, *holderID));
if (!sleMpt)
return tecINTERNAL;

std::uint32_t const flagsIn = sleMpt->getFieldU32(sfFlags);
std::uint32_t flagsOut = flagsIn;

// Issuer wants to unauthorize the holder, unset lsfMPTAuthorized on
// their MPToken
if (txFlags & tfMPTUnauthorize)
flagsOut &= ~lsfMPTAuthorized;
// Issuer wants to authorize a holder, set lsfMPTAuthorized on their
// MPToken
else
flagsOut |= lsfMPTAuthorized;

if (flagsIn != flagsOut)
sleMpt->setFieldU32(sfFlags, flagsOut);

view().update(sleMpt);
return tesSUCCESS;
}

// If the account that submitted the tx is a holder
// Note: `account_` is holder's account
// `holderID` is NOT used
if (holderID)
return tecINTERNAL;

// When a holder wants to unauthorize/delete a MPT, the ledger must
// - delete mptokenKey from both owner and mpt directories
// - delete the MPToken
if (txFlags & tfMPTUnauthorize)
if (!holderID)
{
// When a holder wants to unauthorize/delete a MPT, the ledger must
// - delete mptokenKey from owner directory
// - delete the MPToken
if (ctx_.tx.getFlags() & tfMPTUnauthorize)
{
auto const mptokenKey = keylet::mptoken(mptIssuanceID, account_);
auto const sleMpt = view().peek(mptokenKey);
if (!sleMpt)
return tecINTERNAL;

if (!view().dirRemove(
keylet::ownerDir(account_),
(*sleMpt)[sfOwnerNode],
sleMpt->key(),
false))
return tecINTERNAL;

adjustOwnerCount(view(), sleAcct, -1, j_);

view().erase(sleMpt);
return tesSUCCESS;
}

// A potential holder wants to authorize/hold a mpt, the ledger must:
// - add the new mptokenKey to the owner directory
// - create the MPToken object for the holder
std::uint32_t const uOwnerCount = sleAcct->getFieldU32(sfOwnerCount);
XRPAmount const reserveCreate(
(uOwnerCount < 2) ? XRPAmount(beast::zero)
: view().fees().accountReserve(uOwnerCount + 1));

if (mPriorBalance < reserveCreate)
return tecINSUFFICIENT_RESERVE;

auto const mptokenKey = keylet::mptoken(mptIssuanceID, account_);
auto const sleMpt = view().peek(mptokenKey);
if (!sleMpt)
return tecINTERNAL;

if (!view().dirRemove(
keylet::ownerDir(account_),
(*sleMpt)[sfOwnerNode],
sleMpt->key(),
false))
return tecINTERNAL;
auto const ownerNode = view().dirInsert(
keylet::ownerDir(account_), mptokenKey, describeOwnerDir(account_));

if (!ownerNode)
return tecDIR_FULL;

adjustOwnerCount(view(), sleAcct, -1, j_);
auto mptoken = std::make_shared<SLE>(mptokenKey);
(*mptoken)[sfAccount] = account_;
(*mptoken)[sfMPTokenIssuanceID] = mptIssuanceID;
(*mptoken)[sfFlags] = 0;
(*mptoken)[sfOwnerNode] = *ownerNode;
view().insert(mptoken);

// Update owner count.
adjustOwnerCount(view(), sleAcct, 1, j_);

view().erase(sleMpt);
return tesSUCCESS;
}

// A potential holder wants to authorize/hold a mpt, the ledger must:
// - add the new mptokenKey to both the owner and mpt directries
// - create the MPToken object for the holder
std::uint32_t const uOwnerCount = sleAcct->getFieldU32(sfOwnerCount);
XRPAmount const reserveCreate(
(uOwnerCount < 2) ? XRPAmount(beast::zero)
: view().fees().accountReserve(uOwnerCount + 1));

if (mPriorBalance < reserveCreate)
return tecINSUFFICIENT_RESERVE;
auto const sleMptIssuance = view().read(keylet::mptIssuance(mptIssuanceID));
if (!sleMptIssuance)
return tecINTERNAL;

auto const mptokenKey = keylet::mptoken(mptIssuanceID, account_);
// If the account that submitted this tx is the issuer of the MPT
// Note: `account_` is issuer's account
// `holderID` is holder's account
if (account_ != (*sleMptIssuance)[sfIssuer])
return tecINTERNAL;

auto const ownerNode = view().dirInsert(
keylet::ownerDir(account_), mptokenKey, describeOwnerDir(account_));
auto const sleMpt = view().peek(keylet::mptoken(mptIssuanceID, *holderID));
if (!sleMpt)
return tecINTERNAL;

if (!ownerNode)
return tecDIR_FULL;
std::uint32_t const flagsIn = sleMpt->getFieldU32(sfFlags);
std::uint32_t flagsOut = flagsIn;

auto mptoken = std::make_shared<SLE>(mptokenKey);
(*mptoken)[sfAccount] = account_;
(*mptoken)[sfMPTokenIssuanceID] = mptIssuanceID;
(*mptoken)[sfFlags] = 0;
(*mptoken)[sfOwnerNode] = *ownerNode;
view().insert(mptoken);
// Issuer wants to unauthorize the holder, unset lsfMPTAuthorized on
// their MPToken
if (ctx_.tx.getFlags() & tfMPTUnauthorize)
flagsOut &= ~lsfMPTAuthorized;
// Issuer wants to authorize a holder, set lsfMPTAuthorized on their
// MPToken
else
flagsOut |= lsfMPTAuthorized;

// Update owner count.
adjustOwnerCount(view(), sleAcct, 1, j_);
if (flagsIn != flagsOut)
sleMpt->setFieldU32(sfFlags, flagsOut);

view().update(sleMpt);
return tesSUCCESS;
}

Expand Down
6 changes: 6 additions & 0 deletions src/ripple/ledger/impl/View.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1735,6 +1735,12 @@ rippleMPTCredit(
sle->setFieldU64(
sfOutstandingAmount,
sle->getFieldU64(sfOutstandingAmount) + saAmount.mpt().mpt());

// TODO: Replace maxAmt const with a variable
if (sle->getFieldU64(sfOutstandingAmount) >
(*sle)[~sfMaximumAmount].value_or(0x7FFFFFFFFFFFFFFFull))
return tecMPT_MAX_AMOUNT_EXCEEDED;

view.update(sle);
}
else
Expand Down
3 changes: 2 additions & 1 deletion src/ripple/protocol/TER.h
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,8 @@ enum TECcodes : TERUnderlyingType {
tecXCHAIN_BAD_PUBLIC_KEY_ACCOUNT_PAIR = 185,
tecXCHAIN_CREATE_ACCOUNT_DISABLED = 186,
tecEMPTY_DID = 187,
tecMPTOKEN_EXISTS = 188
tecMPTOKEN_EXISTS = 188,
tecMPT_MAX_AMOUNT_EXCEEDED = 189

};

Expand Down
1 change: 1 addition & 0 deletions src/ripple/protocol/impl/TER.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ transResults()
MAKE_ERROR(tecXCHAIN_CREATE_ACCOUNT_DISABLED, "This bridge does not support account creation."),
MAKE_ERROR(tecEMPTY_DID, "The DID object did not have a URI or DIDDocument field."),
MAKE_ERROR(tecMPTOKEN_EXISTS, "The account already owns the MPToken object."),
MAKE_ERROR(tecMPT_MAX_AMOUNT_EXCEEDED, "The MPT's maximum amount is exceeded."),

MAKE_ERROR(tefALREADY, "The exact transaction was already in this ledger."),
MAKE_ERROR(tefBAD_ADD_AUTH, "Not authorized to add account."),
Expand Down
Loading

0 comments on commit 4bd78ab

Please sign in to comment.