Skip to content

Commit 9a52b83

Browse files
fix comments
1 parent 8d197f8 commit 9a52b83

File tree

5 files changed

+51
-62
lines changed

5 files changed

+51
-62
lines changed

src/test/app/ConfidentialTransfer_test.cpp

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -7,22 +7,6 @@
77

88
namespace xrpl {
99

10-
/*Helper lambda to create a zero-initialized buffer.
11-
WHY THIS IS NEEDED: In C++, xrpl::Buffer(size) allocates uninitialized heap memory.
12-
Because CI runs unit tests sequentially in the same process, uninitialized memory
13-
often recycles "ghost data" (like valid SECP256k1 keys or Pedersen commitments)
14-
left over from previously executed tests.
15-
When testing malformed cryptography paths, passing uninitialized memory might
16-
accidentally supply a valid curve point, causing the ledger's preflight checks
17-
to falsely succeed and return tecBAD_PROOF instead of the expected temMALFORMED.
18-
Explicitly zeroing the buffer guarantees it fails structural validation. */
19-
auto makeZeroBuffer = [](size_t size) {
20-
Buffer b(size);
21-
if (size > 0)
22-
std::memset(b.data(), 0, size);
23-
return b;
24-
};
25-
2610
class ConfidentialTransfer_test : public beast::unit_test::suite
2711
{
2812
// Get a bad ciphertext with valid structure but cryptographic invalid for

src/test/jtx/impl/mpt.cpp

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,6 @@ namespace xrpl {
1414
namespace test {
1515
namespace jtx {
1616

17-
auto makeZeroBuffer = [](size_t size) {
18-
Buffer b(size);
19-
if (size > 0)
20-
std::memset(b.data(), 0, size);
21-
return b;
22-
};
23-
2417
void
2518
mptflags::operator()(Env& env) const
2619
{

src/test/jtx/mpt.h

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,22 @@ class MPTTester;
1818

1919
auto const MPTDEXFlags = tfMPTCanTrade | tfMPTCanTransfer;
2020

21+
/*Helper lambda to create a zero-initialized buffer.
22+
WHY THIS IS NEEDED: In C++, xrpl::Buffer(size) allocates uninitialized heap memory.
23+
Because CI runs unit tests sequentially in the same process, uninitialized memory
24+
often recycles "ghost data" (like valid SECP256k1 keys or Pedersen commitments)
25+
left over from previously executed tests.
26+
When testing malformed cryptography paths, passing uninitialized memory might
27+
accidentally supply a valid curve point, causing the ledger's preflight checks
28+
to falsely succeed and return tecBAD_PROOF instead of the expected temMALFORMED.
29+
Explicitly zeroing the buffer guarantees it fails structural validation. */
30+
auto makeZeroBuffer = [](size_t size) {
31+
Buffer b(size);
32+
if (size > 0)
33+
std::memset(b.data(), 0, size);
34+
return b;
35+
};
36+
2137
// Check flags settings on MPT create
2238
class mptflags
2339
{

src/xrpld/app/tx/detail/InvariantCheck.cpp

Lines changed: 34 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -3332,15 +3332,15 @@ ValidConfidentialMPToken::visitEntry(
33323332
uint192 const id = getMptID(before);
33333333
changes_[id].mptAmountDelta -= before->getFieldU64(sfMPTAmount);
33343334

3335-
// Cannot delete MPToken with non-zero confidential state
3335+
// Cannot delete MPToken with non-zero confidential state or non-zero public amount
33363336
if (isDelete)
33373337
{
3338-
if (before->isFieldPresent(sfIssuerEncryptedBalance) ||
3339-
before->isFieldPresent(sfConfidentialBalanceInbox) ||
3340-
before->isFieldPresent(sfConfidentialBalanceSpending))
3341-
{
3338+
bool const hasPublicBalance = before->getFieldU64(sfMPTAmount) > 0;
3339+
bool const hasEncryptedFields = before->isFieldPresent(sfConfidentialBalanceSpending) ||
3340+
before->isFieldPresent(sfConfidentialBalanceInbox) || before->isFieldPresent(sfIssuerEncryptedBalance);
3341+
3342+
if (hasPublicBalance || hasEncryptedFields)
33423343
changes_[id].deletedWithEncrypted = true;
3343-
}
33443344
}
33453345
}
33463346

@@ -3378,45 +3378,36 @@ ValidConfidentialMPToken::visitEntry(
33783378
if (after && after->getType() == ltMPTOKEN_ISSUANCE)
33793379
{
33803380
uint192 const id = getMptID(after);
3381-
if (after->isFieldPresent(sfConfidentialOutstandingAmount))
3382-
changes_[id].coaDelta += after->getFieldU64(sfConfidentialOutstandingAmount);
3383-
changes_[id].outstandingDelta += after->getFieldU64(sfOutstandingAmount);
3384-
changes_[id].issuance = after;
3381+
auto& change = changes_[id];
3382+
3383+
bool const hasCOA = after->isFieldPresent(sfConfidentialOutstandingAmount);
3384+
std::uint64_t const coa = (*after)[~sfConfidentialOutstandingAmount].value_or(0);
3385+
std::uint64_t const oa = after->getFieldU64(sfOutstandingAmount);
3386+
3387+
if (hasCOA)
3388+
change.coaDelta += coa;
3389+
3390+
change.outstandingDelta += oa;
3391+
change.issuance = after;
33853392

33863393
// COA <= OutstandingAmount
3387-
std::uint64_t const coa = after->isFieldPresent(sfConfidentialOutstandingAmount)
3388-
? after->getFieldU64(sfConfidentialOutstandingAmount)
3389-
: 0;
3390-
std::uint64_t const outstanding = after->getFieldU64(sfOutstandingAmount);
3391-
if (coa > outstanding)
3392-
{
3393-
changes_[id].badCOA = true;
3394-
}
3394+
if (coa > oa)
3395+
change.badCOA = true;
33953396
}
33963397

33973398
if (before && after && before->getType() == ltMPTOKEN && after->getType() == ltMPTOKEN)
33983399
{
33993400
uint192 const id = getMptID(after);
34003401

3401-
auto const getSpending = [](std::shared_ptr<SLE const> const& sle) -> std::optional<Blob> {
3402-
if (sle->isFieldPresent(sfConfidentialBalanceSpending))
3403-
return sle->getFieldVL(sfConfidentialBalanceSpending);
3404-
return std::nullopt;
3405-
};
3406-
3407-
auto const getVersion = [](std::shared_ptr<SLE const> const& sle) -> std::optional<std::uint32_t> {
3408-
if (sle->isFieldPresent(sfConfidentialBalanceVersion))
3409-
return sle->getFieldU32(sfConfidentialBalanceVersion);
3410-
return std::nullopt;
3411-
};
3412-
34133402
// sfConfidentialBalanceVersion must change when spending changes
3414-
auto const spendingBefore = getSpending(before);
3415-
auto const spendingAfter = getSpending(after);
3403+
auto const spendingBefore = (*before)[~sfConfidentialBalanceSpending];
3404+
auto const spendingAfter = (*after)[~sfConfidentialBalanceSpending];
3405+
auto const versionBefore = (*before)[~sfConfidentialBalanceVersion];
3406+
auto const versionAfter = (*after)[~sfConfidentialBalanceVersion];
34163407

34173408
if (spendingBefore.has_value() && spendingBefore != spendingAfter)
34183409
{
3419-
if (getVersion(before) == getVersion(after))
3410+
if (versionBefore == versionAfter)
34203411
{
34213412
changes_[id].badVersion = true;
34223413
}
@@ -3451,11 +3442,7 @@ ValidConfidentialMPToken::finalize(
34513442
// Cannot delete MPToken with non-zero confidential state
34523443
if (checks.deletedWithEncrypted)
34533444
{
3454-
std::uint64_t coa = 0;
3455-
if (issuance->isFieldPresent(sfConfidentialOutstandingAmount))
3456-
coa = issuance->getFieldU64(sfConfidentialOutstandingAmount);
3457-
3458-
if (coa > 0)
3445+
if ((*issuance)[~sfConfidentialOutstandingAmount].value_or(0) > 0)
34593446
{
34603447
JLOG(j.fatal()) << "Invariant failed: MPToken deleted with encrypted fields while COA > 0";
34613448
return false;
@@ -3493,6 +3480,15 @@ ValidConfidentialMPToken::finalize(
34933480
// We only enforce this when Confidential Outstanding Amount changes (Convert, ConvertBack,
34943481
// ConfidentialClawback). This avoids falsely failing on Escrow or AMM operations that lock public tokens
34953482
// outside of ltMPTOKEN.
3483+
// Convert / ConvertBack:
3484+
// - COA and MPTAmount must have opposite deltas, which cancel each other out to zero.
3485+
// - OA remains unchanged.
3486+
// - Therefore, the net delta on both sides of the equation is zero.
3487+
//
3488+
// Clawback:
3489+
// - MPTAmount remains unchanged.
3490+
// - COA and OA must have identical deltas (mirrored on each side).
3491+
// - The equation remains balanced as both sides have equal offsets.
34963492
if (checks.coaDelta != 0)
34973493
{
34983494
if (checks.mptAmountDelta + checks.coaDelta != checks.outstandingDelta)

src/xrpld/app/tx/detail/InvariantCheck.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -703,7 +703,7 @@ class ValidConfidentialMPToken
703703
std::int64_t mptAmountDelta = 0;
704704
std::int64_t coaDelta = 0;
705705
std::int64_t outstandingDelta = 0;
706-
std::shared_ptr<SLE const> issuance;
706+
SLE::const_pointer issuance;
707707
bool deletedWithEncrypted = false;
708708
bool badConsistency = false;
709709
bool badCOA = false;

0 commit comments

Comments
 (0)