Skip to content

Commit

Permalink
Track escrow in recipient's owner directory (RIPD-1523):
Browse files Browse the repository at this point in the history 
Introduce "fix1523" which corrects a minor technical flaw with
the original implementation of the escrow feature.

When creating an escrow, the entry would only be tracked in the
owner directory of the sender; as a result, an escrow recipient
would not be able to detect incoming escrows without monitoring
the ledger in real-time for transactions of interest or without
the sender communicating this information out of band.

With the fix in place, escrows where the recipient differs from
the sender will be listed in the recipient's owner directory as
well.
  • Loading branch information
@nbougalis
nbougalis committed Sep 23, 2017
1 parent 39f9135 commit c7c1b3c
Show file tree
Hide file tree
Showing 10 changed files with 251 additions and 78 deletions.
3 changes: 2 additions & 1 deletion src/ripple/app/main/Amendments.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ supportedAmendments ()
{ "3012E8230864E95A58C60FD61430D7E1B4D3353195F2981DC12B0C7C0950FFAC FlowCross" },
{ "CC5ABAE4F3EC92E94A59B1908C2BE82D2228B6485C00AFF8F22DF930D89C194E SortedDirectories" },
{ "B4D44CC3111ADD964E846FC57760C8B50FFCD5A82C86A72756F6B058DDDF96AD fix1201" },
{ "6C92211186613F9647A89DFFBAB8F94C99D4C7E956D495270789128569177DA1 fix1512" }
{ "6C92211186613F9647A89DFFBAB8F94C99D4C7E956D495270789128569177DA1 fix1512" },
{ "B9E739B8296B4A1BB29BE990B17D66E21B62A300A909F25AC55C22D6C72E1F9D fix1523" }
};
}

Expand Down
38 changes: 36 additions & 2 deletions src/ripple/app/tx/impl/Escrow.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ EscrowCreate::doApply()

ctx_.view().insert(slep);

// Add escrow to owner directory
// Add escrow to sender's owner directory
{
auto page = dirAdd(ctx_.view(), keylet::ownerDir(account), slep->key(),
false, describeOwnerDir(account), ctx_.app.journal ("View"));
Expand All @@ -262,6 +262,21 @@ EscrowCreate::doApply()
(*slep)[sfOwnerNode] = *page;
}

// If it's not a self-send, add escrow to recipient's owner directory.
if (ctx_.view ().rules().enabled(fix1523))
{
auto const dest = ctx_.tx[sfDestination];

if (dest != ctx_.tx[sfAccount])
{
auto page = dirAdd(ctx_.view(), keylet::ownerDir(dest), slep->key(),
false, describeOwnerDir(dest), ctx_.app.journal ("View"));
if (!page)
return tecDIR_FULL;
(*slep)[sfDestinationNode] = *page;
}
}

// Deduct owner's balance, increment owner count
(*sle)[sfBalance] = (*sle)[sfBalance] - ctx_.tx[sfAmount];
(*sle)[sfOwnerCount] = (*sle)[sfOwnerCount] + 1;
Expand Down Expand Up @@ -435,6 +450,16 @@ EscrowFinish::doApply()
return ter;
}

// Remove escrow from recipient's owner directory, if present.
if (ctx_.view ().rules().enabled(fix1523) && (*slep)[~sfDestinationNode])
{
TER const ter = dirDelete(ctx_.view(), true,
(*slep)[sfDestinationNode], keylet::ownerDir((*slep)[sfDestination]),
k.key, false, false, ctx_.app.journal ("View"));
if (! isTesSuccess(ter))
return ter;
}

// NOTE: These payments cannot be used to fund accounts

// Fetch Destination SLE
Expand Down Expand Up @@ -488,7 +513,6 @@ EscrowCancel::doApply()
ctx_.view().info().parentCloseTime.time_since_epoch().count() <=
(*slep)[sfCancelAfter])
return tecNO_PERMISSION;

AccountID const account = (*slep)[sfAccount];

// Remove escrow from owner directory
Expand All @@ -501,6 +525,16 @@ EscrowCancel::doApply()
return ter;
}

// Remove escrow from recipient's owner directory, if present.
if (ctx_.view ().rules().enabled(fix1523) && (*slep)[~sfDestinationNode])
{
TER const ter = dirDelete(ctx_.view(), true,
(*slep)[sfDestinationNode], keylet::ownerDir((*slep)[sfDestination]),
k.key, false, false, ctx_.app.journal ("View"));
if (! isTesSuccess(ter))
return ter;
}

// Transfer amount back to owner, decrement owner count
auto const sle = ctx_.view().peek(
keylet::account(account));
Expand Down
3 changes: 0 additions & 3 deletions src/ripple/ledger/Directory.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,6 @@ class Dir

const_iterator
end() const;

const_iterator
find(uint256 const& page_key, uint256 const& sle_key) const;
};

class Dir::const_iterator
Expand Down
32 changes: 0 additions & 32 deletions src/ripple/ledger/impl/Directory.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,38 +60,6 @@ Dir::end() const ->
return const_iterator(*view_, root_, root_);
}

const_iterator
Dir::find(uint256 const& page_key, uint256 const& sle_key) const
{
if (sle_ == nullptr)
return end();

auto it = const_iterator(*view_, root_, keylet::page(page_key, 0));
if (root_.key == page_key)
{
it.sle_ = sle_;
it.indexes_ = indexes_;
}
else
{
it.sle_ = view_->read(it.page_);
if (it.sle_ == nullptr)
{
it.page_ = root_;
return it;
}
it.indexes_ = &it.sle_->getFieldV256(sfIndexes);
}

it.it_ = std::find(std::begin(*it.indexes_),
std::end(*it.indexes_), sle_key);
if (it.it_ == std::end(*it.indexes_))
return end();

it.index_ = *it.it_;
return it;
}

bool
const_iterator::operator==(const_iterator const& other) const
{
Expand Down
46 changes: 25 additions & 21 deletions src/ripple/protocol/Feature.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,27 +49,30 @@ namespace detail {
class FeatureCollections
{
static constexpr char const* const featureNames[] =
{"MultiSign",
"Tickets",
"TrustSetAuth",
"FeeEscalation",
"OwnerPaysFee",
"CompareFlowV1V2",
"SHAMapV2",
"PayChan",
"Flow",
"CompareTakerFlowCross",
"FlowCross",
"CryptoConditions",
"TickSize",
"fix1368",
"Escrow",
"CryptoConditionsSuite",
"fix1373",
"EnforceInvariants",
"SortedDirectories",
"fix1201",
"fix1512"};
{
"MultiSign",
"Tickets",
"TrustSetAuth",
"FeeEscalation",
"OwnerPaysFee",
"CompareFlowV1V2",
"SHAMapV2",
"PayChan",
"Flow",
"CompareTakerFlowCross",
"FlowCross",
"CryptoConditions",
"TickSize",
"fix1368",
"Escrow",
"CryptoConditionsSuite",
"fix1373",
"EnforceInvariants",
"SortedDirectories",
"fix1201",
"fix1512",
"fix1523"
};

std::vector<uint256> features;
boost::container::flat_map<uint256, std::size_t> featureToIndex;
Expand Down Expand Up @@ -164,6 +167,7 @@ extern uint256 const featureEnforceInvariants;
extern uint256 const featureSortedDirectories;
extern uint256 const fix1201;
extern uint256 const fix1512;
extern uint256 const fix1523;

} // ripple

Expand Down
1 change: 1 addition & 0 deletions src/ripple/protocol/SField.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,7 @@ extern SF_U64 const sfBaseFee;
extern SF_U64 const sfExchangeRate;
extern SF_U64 const sfLowNode;
extern SF_U64 const sfHighNode;
extern SF_U64 const sfDestinationNode;

// 128-bit
extern SF_U128 const sfEmailHash;
Expand Down
1 change: 1 addition & 0 deletions src/ripple/protocol/impl/Feature.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,5 +116,6 @@ uint256 const featureEnforceInvariants = *getRegisteredFeature("EnforceInvariant
uint256 const featureSortedDirectories = *getRegisteredFeature("SortedDirectories");
uint256 const fix1201 = *getRegisteredFeature("fix1201");
uint256 const fix1512 = *getRegisteredFeature("fix1512");
uint256 const fix1523 = *getRegisteredFeature("fix1523");

} // ripple
3 changes: 2 additions & 1 deletion src/ripple/protocol/impl/LedgerFormats.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,8 @@ LedgerFormats::LedgerFormats ()
SOElement (sfDestinationTag, SOE_OPTIONAL) <<
SOElement (sfOwnerNode, SOE_REQUIRED) <<
SOElement (sfPreviousTxnID, SOE_REQUIRED) <<
SOElement (sfPreviousTxnLgrSeq, SOE_REQUIRED);
SOElement (sfPreviousTxnLgrSeq, SOE_REQUIRED) <<
SOElement (sfDestinationNode, SOE_OPTIONAL);

add ("LedgerHashes", ltLEDGER_HASHES)
<< SOElement (sfFirstLedgerSequence, SOE_OPTIONAL) // Remove if we do a ledger restart
Expand Down
17 changes: 9 additions & 8 deletions src/ripple/protocol/impl/SField.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,14 +132,15 @@ SF_U32 const sfSignerListID = make::one<SF_U32::type>(&sfSignerListID,
SF_U32 const sfSettleDelay = make::one<SF_U32::type>(&sfSettleDelay, STI_UINT32, 39, "SettleDelay");

// 64-bit integers
SF_U64 const sfIndexNext = make::one<SF_U64::type>(&sfIndexNext, STI_UINT64, 1, "IndexNext");
SF_U64 const sfIndexPrevious = make::one<SF_U64::type>(&sfIndexPrevious, STI_UINT64, 2, "IndexPrevious");
SF_U64 const sfBookNode = make::one<SF_U64::type>(&sfBookNode, STI_UINT64, 3, "BookNode");
SF_U64 const sfOwnerNode = make::one<SF_U64::type>(&sfOwnerNode, STI_UINT64, 4, "OwnerNode");
SF_U64 const sfBaseFee = make::one<SF_U64::type>(&sfBaseFee, STI_UINT64, 5, "BaseFee");
SF_U64 const sfExchangeRate = make::one<SF_U64::type>(&sfExchangeRate, STI_UINT64, 6, "ExchangeRate");
SF_U64 const sfLowNode = make::one<SF_U64::type>(&sfLowNode, STI_UINT64, 7, "LowNode");
SF_U64 const sfHighNode = make::one<SF_U64::type>(&sfHighNode, STI_UINT64, 8, "HighNode");
SF_U64 const sfIndexNext = make::one<SF_U64::type>(&sfIndexNext, STI_UINT64, 1, "IndexNext");
SF_U64 const sfIndexPrevious = make::one<SF_U64::type>(&sfIndexPrevious, STI_UINT64, 2, "IndexPrevious");
SF_U64 const sfBookNode = make::one<SF_U64::type>(&sfBookNode, STI_UINT64, 3, "BookNode");
SF_U64 const sfOwnerNode = make::one<SF_U64::type>(&sfOwnerNode, STI_UINT64, 4, "OwnerNode");
SF_U64 const sfBaseFee = make::one<SF_U64::type>(&sfBaseFee, STI_UINT64, 5, "BaseFee");
SF_U64 const sfExchangeRate = make::one<SF_U64::type>(&sfExchangeRate, STI_UINT64, 6, "ExchangeRate");
SF_U64 const sfLowNode = make::one<SF_U64::type>(&sfLowNode, STI_UINT64, 7, "LowNode");
SF_U64 const sfHighNode = make::one<SF_U64::type>(&sfHighNode, STI_UINT64, 8, "HighNode");
SF_U64 const sfDestinationNode = make::one<SF_U64::type>(&sfDestinationNode, STI_UINT64, 9, "DestinationNode");

// 128-bit
SF_U128 const sfEmailHash = make::one<SF_U128::type>(&sfEmailHash, STI_HASH128, 1, "EmailHash");
Expand Down
Loading

0 comments on commit c7c1b3c

Please sign in to comment.