Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CAP-0023 implementation #2591

Merged
merged 21 commits into from
Jul 21, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions Builds/VisualStudio/stellar-core.vcxproj
Original file line number Diff line number Diff line change
Expand Up @@ -318,13 +318,16 @@ exit /b 0
<ClCompile Include="..\..\src\historywork\BatchDownloadWork.cpp" />
<ClCompile Include="..\..\src\herder\QuorumIntersectionCheckerImpl.cpp" />
<ClCompile Include="..\..\src\herder\test\QuorumIntersectionTests.cpp" />
<ClCompile Include="..\..\src\ledger\LedgerTxnClaimableBalanceSQL.cpp" />
<ClCompile Include="..\..\src\ledger\test\LedgerCloseMetaStreamTests.cpp" />
<ClCompile Include="..\..\src\main\test\CommandHandlerTests.cpp" />
<ClCompile Include="..\..\src\overlay\SurveyManager.cpp" />
<ClCompile Include="..\..\src\overlay\SurveyMessageLimiter.cpp" />
<ClCompile Include="..\..\src\overlay\test\SurveyManagerTests.cpp" />
<ClCompile Include="..\..\src\overlay\test\SurveyMessageLimiterTests.cpp" />
<ClCompile Include="..\..\src\test\FuzzerImpl.cpp" />
<ClCompile Include="..\..\src\transactions\ClaimClaimableBalanceOpFrame.cpp" />
<ClCompile Include="..\..\src\transactions\CreateClaimableBalanceOpFrame.cpp" />
<ClCompile Include="..\..\src\transactions\FeeBumpTransactionFrame.cpp" />
<ClCompile Include="..\..\src\transactions\PathPaymentOpFrameBase.cpp" />
<ClCompile Include="..\..\src\transactions\PathPaymentStrictReceiveOpFrame.cpp" />
Expand All @@ -338,6 +341,7 @@ exit /b 0
<ClCompile Include="..\..\src\transactions\simulation\TxSimScaleBucketlistWork.cpp" />
<ClCompile Include="..\..\src\transactions\simulation\TxSimTransactionFrame.cpp" />
<ClCompile Include="..\..\src\transactions\simulation\TxSimUtils.cpp" />
<ClCompile Include="..\..\src\transactions\test\ClaimableBalanceTests.cpp" />
<ClCompile Include="..\..\src\transactions\test\FeeBumpTransactionTests.cpp" />
<ClCompile Include="..\..\src\transactions\test\PathPaymentStrictSendTests.cpp" />
<ClCompile Include="..\..\src\transactions\TransactionBridge.cpp" />
Expand Down Expand Up @@ -658,6 +662,8 @@ exit /b 0
<ClInclude Include="..\..\src\overlay\SurveyManager.h" />
<ClInclude Include="..\..\src\overlay\SurveyMessageLimiter.h" />
<ClInclude Include="..\..\src\test\FuzzerImpl.h" />
<ClInclude Include="..\..\src\transactions\ClaimClaimableBalanceOpFrame.h" />
<ClInclude Include="..\..\src\transactions\CreateClaimableBalanceOpFrame.h" />
<ClInclude Include="..\..\src\transactions\FeeBumpTransactionFrame.h" />
<ClInclude Include="..\..\src\transactions\PathPaymentOpFrameBase.h" />
<ClInclude Include="..\..\src\transactions\PathPaymentStrictReceiveOpFrame.h" />
Expand Down
18 changes: 18 additions & 0 deletions Builds/VisualStudio/stellar-core.vcxproj.filters
Original file line number Diff line number Diff line change
Expand Up @@ -1074,6 +1074,18 @@
<ClCompile Include="..\..\lib\tracy\TracyClient.cpp">
<Filter>lib\tracy</Filter>
</ClCompile>
<ClCompile Include="..\..\src\ledger\LedgerTxnClaimableBalanceSQL.cpp">
<Filter>ledger</Filter>
</ClCompile>
<ClCompile Include="..\..\src\transactions\ClaimClaimableBalanceOpFrame.cpp">
<Filter>transactions</Filter>
</ClCompile>
<ClCompile Include="..\..\src\transactions\CreateClaimableBalanceOpFrame.cpp">
<Filter>transactions</Filter>
</ClCompile>
<ClCompile Include="..\..\src\transactions\test\ClaimableBalanceTests.cpp">
<Filter>transactions\tests</Filter>
</ClCompile>
</ItemGroup>
<ItemGroup>
<ClInclude Include="..\..\lib\util\easylogging++.h">
Expand Down Expand Up @@ -1868,6 +1880,12 @@
<ClInclude Include="..\..\lib\fmt\include\fmt\format.h">
<Filter>lib\fmt</Filter>
</ClInclude>
<ClInclude Include="..\..\src\transactions\ClaimClaimableBalanceOpFrame.h">
<Filter>transactions</Filter>
</ClInclude>
<ClInclude Include="..\..\src\transactions\CreateClaimableBalanceOpFrame.h">
<Filter>transactions</Filter>
</ClInclude>
</ItemGroup>
<ItemGroup>
<None Include="..\..\AUTHORS" />
Expand Down
12 changes: 12 additions & 0 deletions docs/db-schema.md
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,18 @@ extension | TEXT | Extension specific to DataEntry (XDR)
ledgerext | TEXT | Extension common to all LedgerEntry types (XDR)
(accountid, dataname) | PRIMARY KEY |

## claimablebalance

Defined in [`src/ledger/LedgerTxnClaimableBalanceSQL.cpp`](/src/ledger/LedgerTxnClaimableBalanceSQL.cpp)

Equivalent to _ClaimableBalanceEntry_

Field | Type | Description
------|------|---------------
balanceid | VARCHAR(48) PRIMARY KEY | This is a ClaimableBalanceID (XDR)
ledgerentry | TEXT NOT NULL | LedgerEntry that contains a ClaimableBalanceEntry (XDR)
lastmodified | INT NOT NULL | lastModifiedLedgerSeq

## txhistory

Defined in [`src/transactions/TransactionFrame.cpp`](/src/transactions/TransactionFrame.cpp)
Expand Down
31 changes: 23 additions & 8 deletions src/bucket/BucketApplicator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,8 @@ BucketApplicator::Counters::reset(VirtualClock::time_point now)
mOfferDelete = 0;
mDataUpsert = 0;
mDataDelete = 0;
mClaimableBalanceUpsert = 0;
mClaimableBalanceDelete = 0;
}

void
Expand All @@ -108,14 +110,15 @@ BucketApplicator::Counters::getRates(VirtualClock::time_point now,
uint64_t& tu_sec, uint64_t& td_sec,
uint64_t& ou_sec, uint64_t& od_sec,
uint64_t& du_sec, uint64_t& dd_sec,
uint64_t& cu_sec, uint64_t& cd_sec,
uint64_t& T_sec, uint64_t& total)
{
VirtualClock::duration dur = now - mStarted;
auto usec = std::chrono::duration_cast<std::chrono::microseconds>(dur);
uint64_t usecs = usec.count() + 1;
total = mAccountUpsert + mAccountDelete + mTrustLineUpsert +
mTrustLineDelete + mOfferUpsert + mOfferDelete + mDataUpsert +
mDataDelete;
mDataDelete + mClaimableBalanceUpsert + mClaimableBalanceDelete;
au_sec = (mAccountUpsert * 1000000) / usecs;
ad_sec = (mAccountDelete * 1000000) / usecs;
tu_sec = (mTrustLineUpsert * 1000000) / usecs;
Expand All @@ -124,6 +127,8 @@ BucketApplicator::Counters::getRates(VirtualClock::time_point now,
od_sec = (mOfferDelete * 1000000) / usecs;
du_sec = (mDataUpsert * 1000000) / usecs;
dd_sec = (mDataDelete * 1000000) / usecs;
cu_sec = (mClaimableBalanceUpsert * 1000000) / usecs;
cd_sec = (mClaimableBalanceDelete * 1000000) / usecs;
T_sec = (total * 1000000) / usecs;
}

Expand All @@ -133,22 +138,25 @@ BucketApplicator::Counters::logInfo(std::string const& bucketName,
VirtualClock::time_point now)
{
uint64_t au_sec, ad_sec, tu_sec, td_sec, ou_sec, od_sec, du_sec, dd_sec,
T_sec, total;
cu_sec, cd_sec, T_sec, total;
getRates(now, au_sec, ad_sec, tu_sec, td_sec, ou_sec, od_sec, du_sec,
dd_sec, T_sec, total);
dd_sec, cu_sec, cd_sec, T_sec, total);
CLOG(INFO, "Bucket") << "Apply-rates for " << total << "-entry bucket "
<< level << "." << bucketName << " au:" << au_sec
<< " ad:" << ad_sec << " tu:" << tu_sec
<< " td:" << td_sec << " ou:" << ou_sec
<< " od:" << od_sec << " du:" << du_sec
<< " dd:" << dd_sec << " T:" << T_sec;
<< " dd:" << dd_sec << " cu:" << cu_sec
<< " cd:" << cd_sec << " T:" << T_sec;
CLOG(INFO, "Bucket") << "Entry-counts for " << total << "-entry bucket "
<< level << "." << bucketName
<< " au:" << mAccountUpsert << " ad:" << mAccountDelete
<< " tu:" << mTrustLineUpsert
<< " td:" << mTrustLineDelete << " ou:" << mOfferUpsert
<< " od:" << mOfferDelete << " du:" << mDataUpsert
<< " dd:" << mDataDelete;
<< " dd:" << mDataDelete
<< " cu:" << mClaimableBalanceUpsert
<< " cd:" << mClaimableBalanceDelete;
}

void
Expand All @@ -157,15 +165,16 @@ BucketApplicator::Counters::logDebug(std::string const& bucketName,
VirtualClock::time_point now)
{
uint64_t au_sec, ad_sec, tu_sec, td_sec, ou_sec, od_sec, du_sec, dd_sec,
T_sec, total;
cu_sec, cd_sec, T_sec, total;
getRates(now, au_sec, ad_sec, tu_sec, td_sec, ou_sec, od_sec, du_sec,
dd_sec, T_sec, total);
dd_sec, cu_sec, cd_sec, T_sec, total);
CLOG(DEBUG, "Bucket") << "Apply-rates for " << total << "-entry bucket "
<< level << "." << bucketName << " au:" << au_sec
<< " ad:" << ad_sec << " tu:" << tu_sec
<< " td:" << td_sec << " ou:" << ou_sec
<< " od:" << od_sec << " du:" << du_sec
<< " dd:" << dd_sec << " T:" << T_sec;
<< " dd:" << dd_sec << " cu:" << cu_sec
<< " cd:" << cd_sec << " T:" << T_sec;
}

void
Expand All @@ -187,6 +196,9 @@ BucketApplicator::Counters::mark(BucketEntry const& e)
case DATA:
++mDataUpsert;
break;
case CLAIMABLE_BALANCE:
++mClaimableBalanceUpsert;
break;
}
}
else
Expand All @@ -205,6 +217,9 @@ BucketApplicator::Counters::mark(BucketEntry const& e)
case DATA:
++mDataDelete;
break;
case CLAIMABLE_BALANCE:
++mClaimableBalanceDelete;
break;
}
}
}
Expand Down
5 changes: 4 additions & 1 deletion src/bucket/BucketApplicator.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,13 @@ class BucketApplicator
uint64_t mOfferDelete;
uint64_t mDataUpsert;
uint64_t mDataDelete;
uint64_t mClaimableBalanceUpsert;
uint64_t mClaimableBalanceDelete;
void getRates(VirtualClock::time_point now, uint64_t& au_sec,
uint64_t& ad_sec, uint64_t& tu_sec, uint64_t& td_sec,
uint64_t& ou_sec, uint64_t& od_sec, uint64_t& du_sec,
uint64_t& dd_sec, uint64_t& T_sec, uint64_t& total);
uint64_t& dd_sec, uint64_t& cu_sec, uint64_t& cd_sec,
uint64_t& T_sec, uint64_t& total);

public:
Counters(VirtualClock::time_point now);
Expand Down
66 changes: 32 additions & 34 deletions src/bucket/LedgerCmp.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,29 @@

namespace stellar
{

template <typename T>
bool
lexCompare(T&& lhs1, T&& rhs1)
{
return lhs1 < rhs1;
}

template <typename T, typename... U>
bool
lexCompare(T&& lhs1, T&& rhs1, U&&... args)
{
if (lhs1 < rhs1)
{
return true;
}
else if (rhs1 < lhs1)
{
return false;
}
return lexCompare(std::forward<U>(args)...);
}

/**
* Compare two LedgerEntries or LedgerKeys for 'identity', not content.
*
Expand Down Expand Up @@ -42,45 +65,20 @@ struct LedgerEntryIdCmp

switch (aty)
{

case ACCOUNT:
return a.account().accountID < b.account().accountID;

case TRUSTLINE:
{
auto const& atl = a.trustLine();
auto const& btl = b.trustLine();
if (atl.accountID < btl.accountID)
return true;
if (btl.accountID < atl.accountID)
return false;
{
return atl.asset < btl.asset;
}
}

return lexCompare(a.trustLine().accountID, b.trustLine().accountID,
a.trustLine().asset, b.trustLine().asset);
case OFFER:
{
auto const& aof = a.offer();
auto const& bof = b.offer();
if (aof.sellerID < bof.sellerID)
return true;
if (bof.sellerID < aof.sellerID)
return false;
return aof.offerID < bof.offerID;
}
return lexCompare(a.offer().sellerID, b.offer().sellerID,
a.offer().offerID, b.offer().offerID);
case DATA:
{
auto const& ad = a.data();
auto const& bd = b.data();
if (ad.accountID < bd.accountID)
return true;
if (bd.accountID < ad.accountID)
return false;
{
return ad.dataName < bd.dataName;
}
}
return lexCompare(a.data().accountID, b.data().accountID,
a.data().dataName, b.data().dataName);
case CLAIMABLE_BALANCE:
return a.claimableBalance().balanceID <
b.claimableBalance().balanceID;
}
return false;
}
Expand Down
5 changes: 5 additions & 0 deletions src/bucket/test/BucketTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,10 @@ TEST_CASE("merging bucket entries", "[bucket]")
liveEntry.data.data() =
LedgerTestUtils::generateValidDataEntry(10);
break;
case CLAIMABLE_BALANCE:
liveEntry.data.claimableBalance() =
LedgerTestUtils::generateValidClaimableBalanceEntry(10);
break;
default:
abort();
}
Expand All @@ -217,6 +221,7 @@ TEST_CASE("merging bucket entries", "[bucket]")
checkDeadAnnihilatesLive(TRUSTLINE);
checkDeadAnnihilatesLive(OFFER);
checkDeadAnnihilatesLive(DATA);
checkDeadAnnihilatesLive(CLAIMABLE_BALANCE);

SECTION("random dead entries annihilates live entries")
{
Expand Down
3 changes: 3 additions & 0 deletions src/database/Database.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,8 @@ Database::applySchemaUpgrade(unsigned long vers)
// and writes those tables.
addTextColumn("offers", "extension");
addTextColumn("accountdata", "extension");

mApp.getLedgerTxnRoot().dropClaimableBalances();
}
break;
default:
Expand Down Expand Up @@ -653,6 +655,7 @@ Database::initialize()
mApp.getLedgerTxnRoot().dropOffers();
mApp.getLedgerTxnRoot().dropTrustLines();
mApp.getLedgerTxnRoot().dropData();
mApp.getLedgerTxnRoot().dropClaimableBalances();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you forgot to update the schema version and to deal with the schema upgrade

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you need to wait for #2593 to be merged so that you can rebase on top of it: it's already changing the database schema version

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you still need to do this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MonsieurNicolas I just rebased with master to pull in #2593. Is there anything else I need to do to deal with the schema upgrade for cap-0023?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes you need to call mApp.getLedgerTxnRoot().dropClaimableBalances(); from Database::applySchemaUpgrade when upgrading to schema version 13 otherwise the table won't be created

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

}
OverlayManager::dropAll(*this);
PersistentState::dropAll(*this);
Expand Down
5 changes: 5 additions & 0 deletions src/invariant/AccountSubEntriesCountIsValid.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,11 @@ updateChangedSubEntriesCount(
calculateDelta(current, previous);
break;
}
case CLAIMABLE_BALANCE:
{
// claimable balance is not a subentry
break;
}
default:
abort();
}
Expand Down
13 changes: 12 additions & 1 deletion src/invariant/BucketListIsConsistentWithDatabase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ BucketListIsConsistentWithDatabase::checkOnBucketApply(
std::shared_ptr<Bucket const> bucket, uint32_t oldestLedger,
uint32_t newestLedger)
{
uint64_t nAccounts = 0, nTrustLines = 0, nOffers = 0, nData = 0;
uint64_t nAccounts = 0, nTrustLines = 0, nOffers = 0, nData = 0,
nClaimableBalance = 0;
{
LedgerTxn ltx(mApp.getLedgerTxnRoot());

Expand Down Expand Up @@ -134,6 +135,9 @@ BucketListIsConsistentWithDatabase::checkOnBucketApply(
case DATA:
++nData;
break;
case CLAIMABLE_BALANCE:
++nClaimableBalance;
MonsieurNicolas marked this conversation as resolved.
Show resolved Hide resolved
break;
default:
abort();
}
Expand Down Expand Up @@ -178,6 +182,13 @@ BucketListIsConsistentWithDatabase::checkOnBucketApply(
{
return fmt::format(countFormat, "Data", nData, nDataInDb);
}
uint64_t nClaimableBalanceInDb =
ltxRoot.countObjects(CLAIMABLE_BALANCE, range);
if (nClaimableBalanceInDb != nClaimableBalance)
{
return fmt::format(countFormat, "ClaimableBalance", nClaimableBalance,
nClaimableBalanceInDb);
}

return {};
}
Expand Down
18 changes: 18 additions & 0 deletions src/invariant/ConservationOfLumens.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,24 @@ calculateDeltaBalance(std::shared_ptr<LedgerEntry const> const& current,
return (current ? current->data.account().balance : 0) -
(previous ? previous->data.account().balance : 0);
}
if (let == CLAIMABLE_BALANCE)
{
int64_t reserveDelta =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is written as if reserve and amount are mutable, maybe OK in this context (albeit a bit confusing as the code doesn't work at all if asset gets modified) but see my comment in the other invariant on validity

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will be simplified when we rebase CAP-33 on top, because the reserve is not stored in that case. But I the code should work correctly in the case that asset changed. This would obviously be a bug if it were to happen, but for clarity and safety the code should account for it (after all, invariants are all about catching bugs).

(current ? current->data.claimableBalance().reserve : 0) -
(previous ? previous->data.claimableBalance().reserve : 0);

auto const& asset = current ? current->data.claimableBalance().asset
: previous->data.claimableBalance().asset;

if (asset.type() != ASSET_TYPE_NATIVE)
{
return reserveDelta;
}

return ((current ? current->data.claimableBalance().amount : 0) -
(previous ? previous->data.claimableBalance().amount : 0)) +
reserveDelta;
}
return 0;
}

Expand Down
Loading