Skip to content

fix: Assorted AI comments #7569

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.

Sign up for GitHub

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

Open
mvadari wants to merge 8 commits into
base: xrplf/sponsor
Choose a base branch
from
Open

fix: Assorted AI comments #7569

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
a6f21a0
fix alpha order
mvadari Jun 17, 2026
a701197
add ledger_entry tests
mvadari Jun 17, 2026
bfe7329
actually run invariant tests
mvadari Jun 17, 2026
31a6411
fix tests
mvadari Jun 17, 2026
9b387fc
fix log line
mvadari Jun 17, 2026
066658d
Merge branch 'xrplf/sponsor' into mvadari/sponsor/fix-ledger-entry
mvadari Jun 17, 2026
d32eacd
Merge branch 'xrplf/sponsor' into mvadari/sponsor/fix-ledger-entry
mvadari Jun 17, 2026
0ba98dd
simplify TxFlags changes (overengineered)
mvadari Jun 17, 2026
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
2 changes: 1 addition & 1 deletion src/libxrpl/ledger/helpers/NFTokenHelpers.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -753,7 +753,7 @@ repairNFTokenDirectoryLinks(ApplyView& view, AccountID const& owner)
{
Throw<std::runtime_error>(
"NFTokenPage directory for " + to_string(owner) +
" cannot be repaired. std::unexpected link problem.");
" cannot be repaired. Unexpected link problem.");
}
newPrev->at(sfNextPageMin) = nextPage->key();
view.update(newPrev);
Expand Down
12 changes: 6 additions & 6 deletions src/libxrpl/tx/invariants/SponsorshipInvariant.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,17 +111,17 @@ SponsorshipOwnerCountsMatch::finalize(
return false;
}

if (deltaSponsoredObjectOwnerCount_ != deltaSponsoredOwnerCount_)
if (invalidOwnerCountLessThanSponsoredOwnerCount_ > 0)

@mvadari mvadari Jun 17, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This diff is hard to read, I just swapped the two checks

@oleks-rip oleks-rip Jun 17, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

GetANameThatNotSoLongOrMaybe2Instead

{
JLOG(j.fatal()) << "Invariant failed: SponsoredObjectOwnerCount does not "
"equal SponsoredOwnerCount delta.";
JLOG(j.fatal())
<< "Invariant failed: OwnerCount must be greater than or equal to SponsoredOwnerCount.";
return false;
}

if (invalidOwnerCountLessThanSponsoredOwnerCount_ > 0)
if (deltaSponsoredObjectOwnerCount_ != deltaSponsoredOwnerCount_)
{
JLOG(j.fatal())
<< "Invariant failed: OwnerCount must be greater than or equal to SponsoredOwnerCount.";
JLOG(j.fatal()) << "Invariant failed: SponsoredObjectOwnerCount does not "
"equal SponsoredOwnerCount delta.";
return false;
}

Expand Down
1 change: 1 addition & 0 deletions src/test/app/Invariants_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5077,6 +5077,7 @@ class Invariants_test : public beast::unit_test::Suite
testInvariantOverwrite(defaultAmendments() - fixCleanup3_1_3);
testVaultComputeCoarsestScale();
testAMM();
testSponsorship();
}
};

Expand Down
58 changes: 57 additions & 1 deletion src/test/rpc/LedgerEntry_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include <test/jtx/offer.h>
#include <test/jtx/pay.h>
#include <test/jtx/permissioned_domains.h>
#include <test/jtx/sponsor.h>
#include <test/jtx/ticket.h>
#include <test/jtx/token.h>
#include <test/jtx/txflags.h>
Expand Down Expand Up @@ -93,6 +94,8 @@ std::vector<std::pair<json::StaticString, FieldType>> gMappings{
{jss::oracle_document_id, FieldType::UInt32Field},
{jss::owner, FieldType::AccountField},
{jss::seq, FieldType::UInt32Field},
{jss::sponsor, FieldType::AccountField},
{jss::sponsee, FieldType::AccountField},
{jss::subject, FieldType::AccountField},
{jss::ticket_seq, FieldType::UInt32Field},
};
Expand All @@ -107,7 +110,7 @@ getFieldType(json::StaticString fieldName)
return it->second;
}

Throw<std::runtime_error>("`mappings` is missing field " + std::string(fieldName.cStr()));
Throw<std::runtime_error>("`gMappings` is missing field " + std::string(fieldName.cStr()));
}

std::string
Expand Down Expand Up @@ -1886,6 +1889,58 @@ class LedgerEntry_test : public beast::unit_test::Suite
runLedgerEntryTest(env, jss::signer_list);
}

void
testSponsorship()
{
testcase("Sponsorship");

using namespace test::jtx;

Env env{*this};
Account const alice{"alice"};
Account const bob{"bob"};
env.fund(XRP(10000), alice, bob);
env.close();
env(sponsor::set(alice, 0), sponsor::SponseeAcc(bob));
env.close();
std::string const ledgerHash{to_string(env.closed()->header().hash)};
std::string sponsorshipIndex;

@yinyiqian1 yinyiqian1 Jun 18, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It is cleaner not to carry mutable state (sponsorshipIndex) between blocks.
It is currently being set in block 1, and being reused in block 2.

We can initialize it with the value, something like: auto const sponsorshipIndex = to_string(keylet::sponsor(alice.id(), bob.id()).key);

and verify it in both block 1 and block 2.

{
// Request by sponsor and sponsee.
json::Value jvParams;
jvParams[jss::sponsorship][jss::sponsor] = alice.human();
jvParams[jss::sponsorship][jss::sponsee] = bob.human();
jvParams[jss::ledger_hash] = ledgerHash;
json::Value const jrr =

@yinyiqian1 yinyiqian1 Jun 18, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: auto

env.rpc("json", "ledger_entry", to_string(jvParams))[jss::result];
BEAST_EXPECT(jrr[jss::node][sfLedgerEntryType.jsonName] == jss::Sponsorship);
BEAST_EXPECT(jrr[jss::node][sfOwner.jsonName] == alice.human());
BEAST_EXPECT(jrr[jss::node][sfSponsee.jsonName] == bob.human());
sponsorshipIndex = jrr[jss::node][jss::index].asString();
}
{
// Request by index.
json::Value jvParams;
jvParams[jss::sponsorship] = sponsorshipIndex;
jvParams[jss::ledger_hash] = ledgerHash;
json::Value const jrr =

@yinyiqian1 yinyiqian1 Jun 18, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: auto

env.rpc("json", "ledger_entry", to_string(jvParams))[jss::result];
BEAST_EXPECT(jrr[jss::node][sfLedgerEntryType.jsonName] == jss::Sponsorship);
BEAST_EXPECT(jrr[jss::node][sfOwner.jsonName] == alice.human());
BEAST_EXPECT(jrr[jss::node][sfSponsee.jsonName] == bob.human());
}
{
// Check all malformed cases.
runLedgerEntryTest(
env,
jss::sponsorship,
{
{.fieldName = jss::sponsor, .malformedErrorMsg = "malformedSponsor"},
{.fieldName = jss::sponsee, .malformedErrorMsg = "malformedSponsee"},
});
}
}

void
testTicket()
{
Expand Down Expand Up @@ -2678,6 +2733,7 @@ class LedgerEntry_test : public beast::unit_test::Suite
testPayChan();
testRippleState();
testSignerList();
testSponsorship();
testTicket();
testDID();
testInvalidOracleLedgerEntry();
Expand Down
46 changes: 23 additions & 23 deletions src/xrpld/rpc/handlers/ledger/LedgerEntry.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -720,7 +720,7 @@ parseSignerList(
}

static std::expected<uint256, json::Value>
parseTicket(
parseSponsorship(

@mvadari mvadari Jun 17, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This diff is hard to read, I just properly alphabetized the location of parseSponsorship

json::Value const& params,
json::StaticString const fieldName,
[[maybe_unused]] unsigned const apiVersion)
Expand All @@ -730,20 +730,21 @@ parseTicket(
return parseObjectID(params, fieldName);
}

auto const id = LedgerEntryHelpers::requiredAccountID(params, jss::account, "malformedAddress");
if (!id)
return std::unexpected(id.error());
auto const sponsorAccountID =
LedgerEntryHelpers::requiredAccountID(params, jss::sponsor, "malformedSponsor");
if (!sponsorAccountID)
return std::unexpected(sponsorAccountID.error());

auto const seq =
LedgerEntryHelpers::requiredUInt32(params, jss::ticket_seq, "malformedRequest");
if (!seq)
return std::unexpected(seq.error());
auto const sponseeAccountID =
LedgerEntryHelpers::requiredAccountID(params, jss::sponsee, "malformedSponsee");
if (!sponseeAccountID)
return std::unexpected(sponseeAccountID.error());

return getTicketIndex(*id, *seq);
return keylet::sponsor(*sponsorAccountID, *sponseeAccountID).key;
}

static std::expected<uint256, json::Value>
parseVault(
parseTicket(
json::Value const& params,
json::StaticString const fieldName,
[[maybe_unused]] unsigned const apiVersion)
Expand All @@ -753,19 +754,20 @@ parseVault(
return parseObjectID(params, fieldName);
}

auto const id = LedgerEntryHelpers::requiredAccountID(params, jss::owner, "malformedOwner");
auto const id = LedgerEntryHelpers::requiredAccountID(params, jss::account, "malformedAddress");
if (!id)
return std::unexpected(id.error());

auto const seq = LedgerEntryHelpers::requiredUInt32(params, jss::seq, "malformedRequest");
auto const seq =
LedgerEntryHelpers::requiredUInt32(params, jss::ticket_seq, "malformedRequest");
if (!seq)
return std::unexpected(seq.error());

return keylet::vault(*id, *seq).key;
return getTicketIndex(*id, *seq);
}

static std::expected<uint256, json::Value>
parseSponsorship(
parseVault(
json::Value const& params,
json::StaticString const fieldName,
[[maybe_unused]] unsigned const apiVersion)
Expand All @@ -775,17 +777,15 @@ parseSponsorship(
return parseObjectID(params, fieldName);
}

auto const sponsorAccountID =
LedgerEntryHelpers::requiredAccountID(params, jss::sponsor, "malformedSponsor");
if (!sponsorAccountID)
return std::unexpected(sponsorAccountID.error());
auto const id = LedgerEntryHelpers::requiredAccountID(params, jss::owner, "malformedOwner");
if (!id)
return std::unexpected(id.error());

auto const sponseeAccountID =
LedgerEntryHelpers::requiredAccountID(params, jss::sponsee, "malformedSponsee");
if (!sponseeAccountID)
return std::unexpected(sponseeAccountID.error());
auto const seq = LedgerEntryHelpers::requiredUInt32(params, jss::seq, "malformedRequest");
if (!seq)
return std::unexpected(seq.error());

return keylet::sponsor(*sponsorAccountID, *sponseeAccountID).key;
return keylet::vault(*id, *seq).key;
}

static std::expected<uint256, json::Value>
Expand Down
Loading