Skip to content

Commit

Permalink
XRPLF review of XLS34 (XRPLF#69)
Browse files Browse the repository at this point in the history 
* add amendment guard for sfTransferRate

* make `sfTransferRate` optional

* split tests into existing / xls34

* make variables `const`

* clang-format

* chage error code

* guard optional `sfTransferRate`

* rename tests

* Guard Optional sfTransferRate

* clang-format

* fixup tests
  • Loading branch information
@dangell7
dangell7 authored Jul 17, 2023
1 parent 5aec64c commit c36683a
Show file tree
Hide file tree
Showing 8 changed files with 645 additions and 1,513 deletions.
40 changes: 25 additions & 15 deletions src/ripple/app/tx/impl/Escrow.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ EscrowCreate::doApply()
auto const balance = STAmount((*sle)[sfBalance]).xrp();
auto const reserve =
ctx_.view().fees().accountReserve((*sle)[sfOwnerCount] + 1);
bool isIssuer = amount.getIssuer() == account;
bool const isIssuer = amount.getIssuer() == account;

if (balance < reserve)
return tecINSUFFICIENT_RESERVE;
Expand All @@ -237,7 +237,7 @@ EscrowCreate::doApply()
if (!ctx_.view().rules().enabled(featurePaychanAndEscrowForTokens))
return temDISABLED;

TER result = trustTransferAllowed(
TER const result = trustTransferAllowed(
ctx_.view(),
{account, ctx_.tx[sfDestination]},
amount.issue(),
Expand All @@ -263,7 +263,7 @@ EscrowCreate::doApply()
return tecNO_LINE;

{
TER result = trustAdjustLockedBalance(
TER const result = trustAdjustLockedBalance(
ctx_.view(), sleLine, amount, 1, ctx_.journal, DryRun);

JLOG(ctx_.journal.trace())
Expand Down Expand Up @@ -296,20 +296,24 @@ EscrowCreate::doApply()

// Create escrow in ledger. Note that we we use the value from the
// sequence or ticket. For more explanation see comments in SeqProxy.h.
auto xferRate = transferRate(view(), amount.getIssuer());
Keylet const escrowKeylet =
keylet::escrow(account, seqID(ctx_));
auto const slep = std::make_shared<SLE>(escrowKeylet);
(*slep)[sfAmount] = ctx_.tx[sfAmount];
(*slep)[sfAccount] = account;
(*slep)[sfTransferRate] = xferRate.value;
(*slep)[~sfCondition] = ctx_.tx[~sfCondition];
(*slep)[~sfSourceTag] = ctx_.tx[~sfSourceTag];
(*slep)[sfDestination] = ctx_.tx[sfDestination];
(*slep)[~sfCancelAfter] = ctx_.tx[~sfCancelAfter];
(*slep)[~sfFinishAfter] = ctx_.tx[~sfFinishAfter];
(*slep)[~sfDestinationTag] = ctx_.tx[~sfDestinationTag];

if (ctx_.view().rules().enabled(featurePaychanAndEscrowForTokens))
{
auto const xferRate = transferRate(view(), amount.getIssuer());
(*slep)[~sfTransferRate] = xferRate.value;
}

ctx_.view().insert(slep);

// Add escrow to sender's owner directory
Expand Down Expand Up @@ -346,7 +350,7 @@ EscrowCreate::doApply()
return tecNO_LINE;

// do the lock-up for real now
TER result = trustAdjustLockedBalance(
TER const result = trustAdjustLockedBalance(
ctx_.view(), sleLine, amount, 1, ctx_.journal, WetRun);

JLOG(ctx_.journal.trace())
Expand Down Expand Up @@ -469,7 +473,7 @@ EscrowFinish::doApply()

AccountID const account = (*slep)[sfAccount];
auto const sle = ctx_.view().peek(keylet::account(account));
auto amount = slep->getFieldAmount(sfAmount);
auto const amount = slep->getFieldAmount(sfAmount);

// If a cancel time is present, a finish operation should only succeed prior
// to that time. fix1571 corrects a logic error in the check that would make
Expand Down Expand Up @@ -576,16 +580,19 @@ EscrowFinish::doApply()
if (!ctx_.view().rules().enabled(featurePaychanAndEscrowForTokens))
return temDISABLED;

if (!slep->isFieldPresent(sfTransferRate))
return tecINTERNAL;

Rate lockedRate = ripple::Rate(slep->getFieldU32(sfTransferRate));
auto issuerAccID = amount.getIssuer();
auto const issuerAccID = amount.getIssuer();
auto const xferRate = transferRate(view(), issuerAccID);
// update if issuer rate is less than locked rate
if (xferRate < lockedRate)
lockedRate = xferRate;

// perform a dry run of the transfer before we
// change anything on-ledger
TER result = trustTransferLockedBalance(
TER const result = trustTransferLockedBalance(
ctx_.view(),
account_, // txn signing account
sle, // src account
Expand Down Expand Up @@ -632,8 +639,11 @@ EscrowFinish::doApply()
else
{
// compute transfer fee, if any
if (!slep->isFieldPresent(sfTransferRate))
return tecINTERNAL;

Rate lockedRate = ripple::Rate(slep->getFieldU32(sfTransferRate));
auto issuerAccID = amount.getIssuer();
auto const issuerAccID = amount.getIssuer();
auto const xferRate = transferRate(view(), issuerAccID);
// update if issuer rate is less than locked rate
if (xferRate < lockedRate)
Expand All @@ -642,7 +652,7 @@ EscrowFinish::doApply()
// all the significant complexity of checking the validity of this
// transfer and ensuring the lines exist etc is hidden away in this
// function, all we need to do is call it and return if unsuccessful.
TER result = trustTransferLockedBalance(
TER const result = trustTransferLockedBalance(
ctx_.view(),
account_, // txn signing account
sle, // src account
Expand Down Expand Up @@ -733,8 +743,8 @@ EscrowCancel::doApply()

AccountID const account = (*slep)[sfAccount];
auto const sle = ctx_.view().peek(keylet::account(account));
auto amount = slep->getFieldAmount(sfAmount);
bool isIssuer = amount.getIssuer() == account;
auto const amount = slep->getFieldAmount(sfAmount);
bool const isIssuer = amount.getIssuer() == account;

std::shared_ptr<SLE> sleLine;

Expand All @@ -750,7 +760,7 @@ EscrowCancel::doApply()
account, amount.getIssuer(), amount.getCurrency()));

// dry run before we make any changes to ledger
if (TER result = trustAdjustLockedBalance(
if (TER const result = trustAdjustLockedBalance(
ctx_.view(), sleLine, -amount, -1, ctx_.journal, DryRun);
result != tesSUCCESS)
return result;
Expand Down Expand Up @@ -794,7 +804,7 @@ EscrowCancel::doApply()
if (!isIssuer)
{
// unlock previously locked tokens from source line
TER result = trustAdjustLockedBalance(
TER const result = trustAdjustLockedBalance(
ctx_.view(), sleLine, -amount, -1, ctx_.journal, WetRun);

JLOG(ctx_.journal.trace())
Expand Down
2 changes: 1 addition & 1 deletion src/ripple/app/tx/impl/InvariantCheck.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ NoZeroEscrow::finalize(
if (bad_ && rv.rules().enabled(featurePaychanAndEscrowForTokens) &&
txn.isFieldPresent(sfTransactionType))
{
uint16_t tt = txn.getFieldU16(sfTransactionType);
uint16_t const tt = txn.getFieldU16(sfTransactionType);
if (tt == ttESCROW_CANCEL || tt == ttESCROW_FINISH)
return true;

Expand Down
87 changes: 49 additions & 38 deletions src/ripple/app/tx/impl/PayChan.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ closeChannel(
keylet::line(src, amount.getIssuer(), amount.getCurrency()));

// dry run
TER result =
TER const result =
trustAdjustLockedBalance(view, sleLine, -amount, -1, j, DryRun);

JLOG(j.trace()) << "closeChannel: trustAdjustLockedBalance(dry) result="
Expand Down Expand Up @@ -178,7 +178,7 @@ closeChannel(
(*sle)[sfBalance] = (*sle)[sfBalance] + amount;
else
{
TER result =
TER const result =
trustAdjustLockedBalance(view, sleLine, -amount, -1, j, WetRun);

JLOG(j.trace()) << "closeChannel: trustAdjustLockedBalance(wet) result="
Expand Down Expand Up @@ -257,7 +257,7 @@ PayChanCreate::preclaim(PreclaimContext const& ctx)
return tecINSUFFICIENT_RESERVE;

auto const dst = ctx.tx[sfDestination];
bool isIssuer = amount.getIssuer() == account;
bool const isIssuer = amount.getIssuer() == account;

// Check reserve and funds availability
if (isXRP(amount) && balance < reserve + amount)
Expand All @@ -272,7 +272,7 @@ PayChanCreate::preclaim(PreclaimContext const& ctx)
// check for any possible bars to a channel existing
// between these accounts for this asset
{
TER result = trustTransferAllowed(
TER const result = trustTransferAllowed(
ctx.view, {account, dst}, amount.issue(), ctx.j);
JLOG(ctx.j.trace())
<< "PayChanCreate::preclaim trustTransferAllowed result="
Expand All @@ -287,7 +287,7 @@ PayChanCreate::preclaim(PreclaimContext const& ctx)
{
auto sleLine = ctx.view.read(keylet::line(
account, amount.getIssuer(), amount.getCurrency()));
TER result = trustAdjustLockedBalance(
TER const result = trustAdjustLockedBalance(
ctx.view, sleLine, amount, 1, ctx.j, DryRun);
JLOG(ctx.j.trace()) << "PayChanCreate::preclaim "
"trustAdjustLockedBalance(dry) result="
Expand Down Expand Up @@ -334,8 +334,7 @@ PayChanCreate::doApply()
auto const dst = ctx_.tx[sfDestination];

STAmount const amount{ctx_.tx[sfAmount]};
bool isIssuer = amount.getIssuer() == account;
auto xferRate = transferRate(view(), amount.getIssuer());
bool const isIssuer = amount.getIssuer() == account;

// Create PayChan in ledger.
//
Expand All @@ -353,12 +352,17 @@ PayChanCreate::doApply()
(*slep)[sfAccount] = account;
(*slep)[sfDestination] = dst;
(*slep)[sfSettleDelay] = ctx_.tx[sfSettleDelay];
(*slep)[sfTransferRate] = xferRate.value;
(*slep)[sfPublicKey] = ctx_.tx[sfPublicKey];
(*slep)[~sfCancelAfter] = ctx_.tx[~sfCancelAfter];
(*slep)[~sfSourceTag] = ctx_.tx[~sfSourceTag];
(*slep)[~sfDestinationTag] = ctx_.tx[~sfDestinationTag];

if (ctx_.view().rules().enabled(featurePaychanAndEscrowForTokens))
{
auto xferRate = transferRate(view(), amount.getIssuer());
(*slep)[~sfTransferRate] = xferRate.value;
}

ctx_.view().insert(slep);

// Add PayChan to owner directory
Expand Down Expand Up @@ -398,7 +402,7 @@ PayChanCreate::doApply()
if (!sleLine)
return tecNO_LINE;

TER result = trustAdjustLockedBalance(
TER const result = trustAdjustLockedBalance(
ctx_.view(), sleLine, amount, 1, ctx_.journal, WetRun);

JLOG(ctx_.journal.trace())
Expand Down Expand Up @@ -470,24 +474,28 @@ PayChanFund::doApply()
AccountID const src = (*slep)[sfAccount];
auto const txAccount = ctx_.tx[sfAccount];
auto const expiration = (*slep)[~sfExpiration];
bool isIssuer = amount.getIssuer() == txAccount;
// auto const chanFunds = (*slep)[sfAmount];

// adjust transfer rate
Rate lockedRate = ripple::Rate(slep->getFieldU32(sfTransferRate));
auto issuerAccID = amount.getIssuer();
auto const xferRate = transferRate(view(), issuerAccID);
// update if issuer rate less than locked rate
if (xferRate < lockedRate)
(*slep)[sfTransferRate] = xferRate.value;
// throw if issuer rate greater than locked rate
if (xferRate > lockedRate)
return temBAD_TRANSFER_RATE;
bool const isIssuer = amount.getIssuer() == txAccount;

// if this is a Fund operation on an IOU then perform a dry run here
if (!isXRP(amount) &&
ctx_.view().rules().enabled(featurePaychanAndEscrowForTokens))
{
// adjust transfer rate
if (!slep->isFieldPresent(sfTransferRate))
return tecINTERNAL;

Rate lockedRate = ripple::Rate(slep->getFieldU32(sfTransferRate));
auto const issuerAccID = amount.getIssuer();
auto const xferRate = transferRate(view(), issuerAccID);

// update if issuer rate less than locked rate
if (xferRate < lockedRate)
(*slep)[~sfTransferRate] = xferRate.value;

// throw if issuer rate greater than locked rate
if (xferRate > lockedRate)
return temBAD_TRANSFER_RATE;

// issuer does not need to lock anything
if (!isIssuer)
{
Expand All @@ -497,7 +505,7 @@ PayChanFund::doApply()
sleLine = ctx_.view().peek(keylet::line(
(*slep)[sfAccount], amount.getIssuer(), amount.getCurrency()));

TER result = trustAdjustLockedBalance(
TER const result = trustAdjustLockedBalance(
ctx_.view(), sleLine, amount, 1, ctx_.journal, DryRun);

JLOG(ctx_.journal.trace())
Expand Down Expand Up @@ -571,7 +579,7 @@ PayChanFund::doApply()
// issuer does not need to lock anything
if (!isIssuer)
{
TER result = trustAdjustLockedBalance(
TER const result = trustAdjustLockedBalance(
ctx_.view(), sleLine, amount, 1, ctx_.journal, WetRun);

JLOG(ctx_.journal.trace())
Expand Down Expand Up @@ -715,11 +723,11 @@ PayChanClaim::doApply()
}

if (reqBalance > chanFunds)
return tecINSUFFICIENT_FUNDS;
return tecUNFUNDED_PAYMENT;

if (reqBalance <= chanBalance)
// nothing requested
return tecINSUFFICIENT_FUNDS;
return tecUNFUNDED_PAYMENT;

auto sled = ctx_.view().peek(keylet::account(dst));
if (!sled)
Expand All @@ -746,17 +754,6 @@ PayChanClaim::doApply()
}
}

// compute transfer fee, if any
Rate lockedRate = ripple::Rate(slep->getFieldU32(sfTransferRate));
auto issuerAccID = chanFunds.getIssuer();
auto const xferRate = transferRate(view(), issuerAccID);
// update if issuer rate is less than locked rate
if (xferRate < lockedRate)
{
(*slep)[sfTransferRate] = xferRate.value;
lockedRate = xferRate;
}

(*slep)[sfBalance] = ctx_.tx[sfBalance];
STAmount const reqDelta = reqBalance - chanBalance;
assert(reqDelta >= beast::zero);
Expand All @@ -770,8 +767,22 @@ PayChanClaim::doApply()
if (!ctx_.view().rules().enabled(featurePaychanAndEscrowForTokens))
return temDISABLED;

// compute transfer fee, if any
if (!slep->isFieldPresent(sfTransferRate))
return tecINTERNAL;

Rate lockedRate = ripple::Rate(slep->getFieldU32(sfTransferRate));
auto const issuerAccID = chanFunds.getIssuer();
auto const xferRate = transferRate(view(), issuerAccID);
// update if issuer rate is less than locked rate
if (xferRate < lockedRate)
{
(*slep)[~sfTransferRate] = xferRate.value;
lockedRate = xferRate;
}

auto sleSrcAcc = ctx_.view().peek(keylet::account(src));
TER result = trustTransferLockedBalance(
TER const result = trustTransferLockedBalance(
ctx_.view(),
txAccount,
sleSrcAcc,
Expand Down
Loading

0 comments on commit c36683a

Please sign in to comment.