Skip to content

Commit

Permalink
[FOLD] Don't pass context to shared functions
Browse files Browse the repository at this point in the history
  • Loading branch information
scottschurr committed Apr 30, 2024
1 parent b3b8b7f commit 9c8d84e
Show file tree
Hide file tree
Showing 5 changed files with 124 additions and 96 deletions.
27 changes: 22 additions & 5 deletions src/ripple/app/tx/impl/NFTokenCreateOffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,15 @@ NFTokenCreateOffer::preflight(PreflightContext const& ctx)
auto const nftFlags = nft::getFlags(ctx.tx[sfNFTokenID]);

// Use implementation shared with NFTokenMint
if (NotTEC notTec = nft::tokenOfferCreatePreflight(ctx, nftFlags, txFlags);
if (NotTEC notTec = nft::tokenOfferCreatePreflight(
ctx.tx[sfAccount],
ctx.tx[sfAmount],
ctx.tx[~sfDestination],
ctx.tx[~sfExpiration],
nftFlags,
ctx.rules,
ctx.tx[~sfOwner],
txFlags);
!isTesSuccess(notTec))
return notTec;

Expand All @@ -68,10 +76,15 @@ NFTokenCreateOffer::preclaim(PreclaimContext const& ctx)

// Use implementation shared with NFTokenMint
return nft::tokenOfferCreatePreclaim(
ctx,
ctx.view,
ctx.tx[sfAccount],
nft::getIssuer(nftokenID),
ctx.tx[sfAmount],
ctx.tx[~sfDestination],
nft::getFlags(nftokenID),
nft::getTransferFee(nftokenID),
ctx.j,
ctx.tx[~sfOwner],
txFlags);
}

Expand All @@ -80,12 +93,16 @@ NFTokenCreateOffer::doApply()
{
// Use implementation shared with NFTokenMint
return nft::tokenOfferCreateApply(
ctx_,
view(),
ctx_.tx[sfAccount],
ctx_.tx[sfAmount],
ctx_.tx[~sfDestination],
ctx_.tx[~sfExpiration],
ctx_.tx.getSeqProxy(),
ctx_.tx[sfNFTokenID],
mPriorBalance,
ctx_.tx.getFlags(),
j_);
j_,
ctx_.tx.getFlags());
}

} // namespace ripple
24 changes: 19 additions & 5 deletions src/ripple/app/tx/impl/NFTokenMint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,12 @@ NFTokenMint::preflight(PreflightContext const& ctx)
// do the validation. We pass tfSellNFToken as the transaction flags
// because a Mint is only allowed to create a sell offer.
if (NotTEC notTec = nft::tokenOfferCreatePreflight(
ctx,
ctx.tx[sfAccount],
ctx.tx[sfAmount],
ctx.tx[~sfDestination],
ctx.tx[~sfExpiration],
extractNFTokenFlagsFromTxFlags(ctx.tx.getFlags()),
tfSellNFToken);
ctx.rules);
!isTesSuccess(notTec))
{
return notTec;
Expand Down Expand Up @@ -189,11 +192,14 @@ NFTokenMint::preclaim(PreclaimContext const& ctx)
// do the validation. We pass tfSellNFToken as the transaction flags
// because a Mint is only allowed to create a sell offer.
if (TER const ter = nft::tokenOfferCreatePreclaim(
ctx,
ctx.view,
ctx.tx[sfAccount],
ctx.tx[~sfIssuer].value_or(ctx.tx[sfAccount]),
ctx.tx[sfAmount],
ctx.tx[~sfDestination],
extractNFTokenFlagsFromTxFlags(ctx.tx.getFlags()),
ctx.tx[~sfTransferFee].value_or(0),
tfSellNFToken);
ctx.j);
!isTesSuccess(ter))
return ter;
}
Expand Down Expand Up @@ -315,7 +321,15 @@ NFTokenMint::doApply()
// the offer. We pass tfSellNFToken as the transaction flags
// because a Mint is only allowed to create a sell offer.
if (TER const ter = nft::tokenOfferCreateApply(
ctx_, view(), nftokenID, mPriorBalance, tfSellNFToken, j_);
view(),
ctx_.tx[sfAccount],
ctx_.tx[sfAmount],
ctx_.tx[~sfDestination],
ctx_.tx[~sfExpiration],
ctx_.tx.getSeqProxy(),
nftokenID,
mPriorBalance,
j_);
!isTesSuccess(ter))
return ter;
}
Expand Down
127 changes: 61 additions & 66 deletions src/ripple/app/tx/impl/details/NFTokenUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -638,49 +638,46 @@ deleteTokenOffer(ApplyView& view, std::shared_ptr<SLE> const& offer)

NotTEC
tokenOfferCreatePreflight(
PreflightContext const& ctx,
AccountID const& acctID,
STAmount const& amount,
std::optional<AccountID> const& dest,
std::optional<std::uint32_t> const& expiration,
std::uint16_t nftFlags,
Rules const& rules,
std::optional<AccountID> const& owner,
std::uint32_t txFlags)
{
bool const isSellOffer = txFlags & tfSellNFToken;
{
// Scope amount. We don't need it for the entire function.
STAmount const amount = ctx.tx[sfAmount];
if (amount.negative() && rules.enabled(fixNFTokenNegOffer))
// An offer for a negative amount makes no sense.
return temBAD_AMOUNT;

if (amount.negative() && ctx.rules.enabled(fixNFTokenNegOffer))
// An offer for a negative amount makes no sense.
if (!isXRP(amount))
{
if (nftFlags & nft::flagOnlyXRP)
return temBAD_AMOUNT;

if (!isXRP(amount))
{
if (nftFlags & nft::flagOnlyXRP)
return temBAD_AMOUNT;

if (!amount)
return temBAD_AMOUNT;
}

// If this is an offer to buy, you must offer something; if it's an
// offer to sell, you can ask for nothing.
if (!isSellOffer && !amount)
if (!amount)
return temBAD_AMOUNT;
}

if (auto exp = ctx.tx[~sfExpiration]; exp == 0)
return temBAD_EXPIRATION;
// If this is an offer to buy, you must offer something; if it's an
// offer to sell, you can ask for nothing.
bool const isSellOffer = txFlags & tfSellNFToken;
if (!isSellOffer && !amount)
return temBAD_AMOUNT;

auto const owner = ctx.tx[~sfOwner];
if (expiration.has_value() && expiration.value() == 0)
return temBAD_EXPIRATION;

// The 'Owner' field must be present when offering to buy, but can't
// be present when selling (it's implicit):
if (owner.has_value() == isSellOffer)
return temMALFORMED;

auto const account = ctx.tx[sfAccount];
if (owner && owner == account)
if (owner && owner == acctID)
return temMALFORMED;

if (auto dest = ctx.tx[~sfDestination])
if (dest)
{
// Some folks think it makes sense for a buy offer to specify a
// specific broker using the Destination field. This change doesn't
Expand All @@ -689,104 +686,100 @@ tokenOfferCreatePreflight(
//
// Prior to fixNFTokenNegOffer any use of the Destination field on
// a buy offer was malformed.
if (!isSellOffer && !ctx.rules.enabled(fixNFTokenNegOffer))
if (!isSellOffer && !rules.enabled(fixNFTokenNegOffer))
return temMALFORMED;

// The destination can't be the account executing the transaction.
if (dest == account)
if (dest == acctID)
return temMALFORMED;
}
return tesSUCCESS;
}

TER
tokenOfferCreatePreclaim(
PreclaimContext const& ctx,
AccountID const& nftIssuer, // Issuer if present, otherwise Account
ReadView const& view,
AccountID const& acctID,
AccountID const& nftIssuer,
STAmount const& amount,
std::optional<AccountID> const& dest,
std::uint16_t nftFlags,
std::uint16_t xferFee,
beast::Journal j,
std::optional<AccountID> const& owner,
std::uint32_t txFlags)
{
bool const isSellOffer = txFlags & tfSellNFToken;
auto const amount = ctx.tx[sfAmount];

if (!(nftFlags & nft::flagCreateTrustLines) && !amount.native() && xferFee)
{
if (!ctx.view.exists(keylet::account(nftIssuer)))
if (!view.exists(keylet::account(nftIssuer)))
return tecNO_ISSUER;

// If the IOU issuer and the NFToken issuer are the same, then that
// issuer does not need a trust line to accept their fee.
if (ctx.view.rules().enabled(featureNFTokenMintOffer))
if (view.rules().enabled(featureNFTokenMintOffer))
{
if (nftIssuer != amount.getIssuer() &&
!ctx.view.read(keylet::line(nftIssuer, amount.issue())))
!view.read(keylet::line(nftIssuer, amount.issue())))
return tecNO_LINE;
}
else if (!ctx.view.exists(keylet::line(nftIssuer, amount.issue())))
else if (!view.exists(keylet::line(nftIssuer, amount.issue())))
{
return tecNO_LINE;
}

if (isFrozen(
ctx.view, nftIssuer, amount.getCurrency(), amount.getIssuer()))
if (isFrozen(view, nftIssuer, amount.getCurrency(), amount.getIssuer()))
return tecFROZEN;
}

AccountID const acctID = ctx.tx[sfAccount];
if (nftIssuer != acctID && !(nftFlags & nft::flagTransferable))
{
auto const root = ctx.view.read(keylet::account(nftIssuer));
auto const root = view.read(keylet::account(nftIssuer));
assert(root);

if (auto minter = (*root)[~sfNFTokenMinter]; minter != acctID)
return tefNFTOKEN_IS_NOT_TRANSFERABLE;
}

if (isFrozen(ctx.view, acctID, amount.getCurrency(), amount.getIssuer()))
if (isFrozen(view, acctID, amount.getCurrency(), amount.getIssuer()))
return tecFROZEN;

// If this is an offer to buy the token, the account must have the
// needed funds at hand; but note that funds aren't reserved and the
// offer may later become unfunded.
if (!isSellOffer)
if ((txFlags & tfSellNFToken) == 0)
{
// After this amendment, we allow an IOU issuer to make a buy offer
// using their own currency.
if (ctx.view.rules().enabled(fixNonFungibleTokensV1_2))
if (view.rules().enabled(fixNonFungibleTokensV1_2))
{
if (accountFunds(
ctx.view,
acctID,
amount,
FreezeHandling::fhZERO_IF_FROZEN,
ctx.j)
view, acctID, amount, FreezeHandling::fhZERO_IF_FROZEN, j)
.signum() <= 0)
return tecUNFUNDED_OFFER;
}
else if (
accountHolds(
ctx.view,
view,
acctID,
amount.getCurrency(),
amount.getIssuer(),
FreezeHandling::fhZERO_IF_FROZEN,
ctx.j)
j)
.signum() <= 0)
return tecUNFUNDED_OFFER;
}

if (auto const destination = ctx.tx[~sfDestination])
if (dest)
{
// If a destination is specified, the destination must already be in
// the ledger.
auto const sleDst = ctx.view.read(keylet::account(*destination));
auto const sleDst = view.read(keylet::account(*dest));

if (!sleDst)
return tecNO_DST;

// check if the destination has disallowed incoming offers
if (ctx.view.rules().enabled(featureDisallowIncoming))
if (view.rules().enabled(featureDisallowIncoming))
{
// flag cannot be set unless amendment is enabled but
// out of an abundance of caution check anyway
Expand All @@ -796,12 +789,12 @@ tokenOfferCreatePreclaim(
}
}

if (auto const owner = ctx.tx[~sfOwner])
if (owner)
{
// Check if the owner (buy offer) has disallowed incoming offers
if (ctx.view.rules().enabled(featureDisallowIncoming))
if (view.rules().enabled(featureDisallowIncoming))
{
auto const sleOwner = ctx.view.read(keylet::account(*owner));
auto const sleOwner = view.read(keylet::account(*owner));

// defensively check
// it should not be possible to specify owner that doesn't exist
Expand All @@ -818,21 +811,23 @@ tokenOfferCreatePreclaim(

TER
tokenOfferCreateApply(
ApplyContext const& ctx,
ApplyView& view,
AccountID const& acctID,
STAmount const& amount,
std::optional<AccountID> const& dest,
std::optional<std::uint32_t> const& expiration,
SeqProxy seqProxy,
uint256 const& nftokenID,
XRPAmount const& priorBalance,
std::uint32_t txFlags,
beast::Journal j)
beast::Journal j,
std::uint32_t txFlags)
{
AccountID const acctID = ctx.tx[sfAccount];

Keylet const acctKeylet = keylet::account(acctID);
if (auto const acct = view.read(acctKeylet);
priorBalance < view.fees().accountReserve((*acct)[sfOwnerCount] + 1))
return tecINSUFFICIENT_RESERVE;

auto const offerID = keylet::nftoffer(acctID, ctx.tx.getSeqProxy().value());
auto const offerID = keylet::nftoffer(acctID, seqProxy.value());

// Create the offer:
{
Expand Down Expand Up @@ -868,16 +863,16 @@ tokenOfferCreateApply(
auto offer = std::make_shared<SLE>(offerID);
(*offer)[sfOwner] = acctID;
(*offer)[sfNFTokenID] = nftokenID;
(*offer)[sfAmount] = ctx.tx[sfAmount];
(*offer)[sfAmount] = amount;
(*offer)[sfFlags] = sleFlags;
(*offer)[sfOwnerNode] = *ownerNode;
(*offer)[sfNFTokenOfferNode] = *offerNode;

if (auto const expiration = ctx.tx[~sfExpiration])
if (expiration)
(*offer)[sfExpiration] = *expiration;

if (auto const destination = ctx.tx[~sfDestination])
(*offer)[sfDestination] = *destination;
if (dest)
(*offer)[sfDestination] = *dest;

view.insert(offer);
}
Expand Down
Loading

0 comments on commit 9c8d84e

Please sign in to comment.