Fix limiting step re-execute bug (RIPD-1368):

The deferred credits table can compute a balance that's different from the
ledger balance.

Syntax:
A number written with no decimal means that number exactly. I.e. "12". A number
written with a decimal means that number has a non-zero digit at the lowest
order digit. I.e. "12.XX" means a number like "12.00000000000005"

Consider the following payment:
alice (USD) -> USD/XRP -> (XRP) Bob
Alice initially has 12.XX USD in her account.
The strand is used to debit alice the following amounts:
1) Debit alice 5
2) Debit alice 0.XX
3) Debit alice 3.XX

The next time the strand is explored, alice has a USD/XRP offer on the books,
and her account is credited:

1) Credit alice 20

When the beginning of the strand is reached, consider what happens when alice is
a limiting step. Calculate how much we can get out the step. According to the
deferred credit table this is:
12.XX - (5 + 0.XX + 3.XX)

This is also limited by alice's balance, which is large thanks to the credit she
received in the book step.

Now that the step has calculated how much we can get out, throw out the
sandbox (the one with the credit), and re-execute. However, the following error
occurs. We asked for 12.XX - (5 + 0.XX + 3.XX). However, the ledger has
calculated that alice has:
((12.XX - 5) - 0.XX) - 3.XX

That's a problem, because that number is smaller. Notice that there are two
precision losing operations in the deferred credits table:
1) The 5 + 0.XX step
2) The 12.XX - (total of debits). (Notice total of debits is < 10)

However, there is only one precision losing operation in the ledger calculation:
1) (Subtotal of 12.XX-5) - 0.XX

That means the calculation for the ledger results in a number that's smaller
than the deferred credits. Flow detects this as a re-execution error.
This commit is contained in:
seelabs
2016-12-19 13:55:28 -05:00
parent b4a16b165b
commit 2c6b0f3193
6 changed files with 72 additions and 2 deletions

View File

@@ -49,7 +49,8 @@ supportedAmendments ()
{ "08DE7D96082187F6E6578530258C77FAABABE4C20474BDB82F04B021F1A68647 PayChan" },
{ "740352F2412A9909880C23A559FCECEDA3BE2126FED62FC7660D628A06927F11 Flow" },
{ "1562511F573A19AE9BD103B5D6B9E01B3B46805AEC5D3C4805C902B514399146 CryptoConditions" },
{ "532651B4FD58DF8922A49BA101AB3E996E5BFBF95A913B3E392504863E63B164 TickSize" }
{ "532651B4FD58DF8922A49BA101AB3E996E5BFBF95A913B3E392504863E63B164 TickSize" },
{ "67431BE4A5F355C45E36EED94B38DFACC1F111F547360F444642512CBC0E18E1 RIPD1368" }
};
}

View File

@@ -344,7 +344,6 @@ bool amendmentRIPD1274 (NetClock::time_point const closeTime);
NetClock::time_point const& amendmentRIPD1298SoTime ();
bool amendmentRIPD1298 (NetClock::time_point const closeTime);
} // ripple
#endif

View File

@@ -21,6 +21,7 @@
#include <ripple/app/paths/impl/AmountSpec.h>
#include <ripple/ledger/PaymentSandbox.h>
#include <ripple/ledger/View.h>
#include <ripple/protocol/Feature.h>
#include <ripple/protocol/SField.h>
#include <ripple/protocol/STAccount.h>
#include <boost/optional.hpp>
@@ -186,15 +187,26 @@ PaymentSandbox::balanceHook (AccountID const& account,
{
auto delta = amount.zeroed ();
auto lastBal = amount;
auto minBal = amount;
for (auto curSB = this; curSB; curSB = curSB->ps_)
{
if (auto adj = curSB->tab_.adjustments (account, issuer, currency))
{
delta += adj->debits;
lastBal = adj->origBalance;
if (lastBal < minBal)
minBal = lastBal;
}
}
adjustedAmt = std::min(amount, lastBal - delta);
if (rules().enabled(featureRIPD1368))
{
// The adjusted amount should never be larger than the balance. In
// some circumstances, it is possible for the deferred credits table
// to compute usable balance just slightly above what the ledger
// calculates (but always less than the actual balance).
adjustedAmt = std::min(adjustedAmt, minBal);
}
if (amendmentRIPD1274 (info ().parentCloseTime))
adjustedAmt.setIssuer(amount.getIssuer());
}

View File

@@ -46,6 +46,7 @@ extern uint256 const featurePayChan;
extern uint256 const featureFlow;
extern uint256 const featureCryptoConditions;
extern uint256 const featureTickSize;
extern uint256 const featureRIPD1368;
} // ripple

View File

@@ -57,5 +57,6 @@ uint256 const featurePayChan = feature("PayChan");
uint256 const featureFlow = feature("Flow");
uint256 const featureCryptoConditions = feature("CryptoConditions");
uint256 const featureTickSize = feature("TickSize");
uint256 const featureRIPD1368 = feature("RIPD1368");
} // ripple

View File

@@ -1379,6 +1379,61 @@ struct Flow_test : public beast::unit_test::suite
}
}
template<class... Features>
void
testReexecuteDirectStep(Features&&... fs)
{
testcase("ReexecuteDirectStep");
using namespace jtx;
Env env(*this, features(fs)...);
auto const alice = Account("alice");
auto const bob = Account("bob");
auto const gw = Account("gw");
auto const USD = gw["USD"];
env.fund(XRP(10000), alice, bob, gw);
env(trust(alice, USD(100)));
env(pay(
gw, alice,
// 12.55....
STAmount{USD.issue(), std::uint64_t(1255555555555555ull), -14, false}));
env(offer(gw,
// 5.0...
STAmount{
USD.issue(), std::uint64_t(5000000000000000ull), -15, false},
XRP(1000)));
env(offer(gw,
// .555...
STAmount{
USD.issue(), std::uint64_t(5555555555555555ull), -16, false},
XRP(10)));
env(offer(gw,
// 4.44....
STAmount{
USD.issue(), std::uint64_t(4444444444444444ull), -15, false},
XRP(.1)));
env(offer(alice,
// 17
STAmount{
USD.issue(), std::uint64_t(1700000000000000ull), -14, false},
XRP(.001)));
// Need to be past this time to see the bug
env.close(amendmentRIPD1274SoTime() +
100 * env.closed()->info().closeTimeResolution);
env(pay(alice, bob, XRP(10000)), path(~XRP), sendmax(USD(100)),
txflags(tfPartialPayment | tfNoRippleDirect));
}
void run() override
{
testDirectStep ();
@@ -1394,6 +1449,7 @@ struct Flow_test : public beast::unit_test::suite
testSelfFundedXRPEndpoint(true);
testUnfundedOffer(true);
testUnfundedOffer(false);
testReexecuteDirectStep(featureFlow, featureRIPD1368);
}
};