Skip to content

test: Add more Sponsor tests#7783

Open
mvadari wants to merge 4 commits into
developfrom
mvadari/sponsor/ai-tests
Open

test: Add more Sponsor tests#7783
mvadari wants to merge 4 commits into
developfrom
mvadari/sponsor/ai-tests

Conversation

@mvadari

@mvadari mvadari commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

High Level Overview of Change

This PR adds more tests for Sponsor-related code.

No change to source code, just tests.

Context of Change

Better code testing

API Impact

N/A

@mvadari mvadari added the DraftRunCI Normally CI does not run on draft PRs. This opts in. label Jul 9, 2026
@mvadari
mvadari force-pushed the mvadari/sponsor/ai-tests branch from 5dbe37d to 9c9b66d Compare July 9, 2026 23:34
@mvadari mvadari changed the title amendment-gate new code add lots of tests Jul 9, 2026
@codecov

codecov Bot commented Jul 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.8%. Comparing base (a24e543) to head (c27ae3c).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           develop   #7783   +/-   ##
=======================================
  Coverage     82.8%   82.8%           
=======================================
  Files         1036    1036           
  Lines        80107   80110    +3     
  Branches      8932    8931    -1     
=======================================
+ Hits         66300   66315   +15     
+ Misses       13798   13786   -12     
  Partials         9       9           

see 7 files with indirect coverage changes

Impacted file tree graph

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mvadari
mvadari changed the base branch from xrplf/sponsor to develop July 15, 2026 14:40
@mvadari mvadari changed the title add lots of tests test: Add more Sponsor tests Jul 15, 2026
@github-actions

Copy link
Copy Markdown

This PR has conflicts, please resolve them in order for the PR to be reviewed.

@mvadari
mvadari force-pushed the mvadari/sponsor/ai-tests branch from 9c9b66d to f234682 Compare July 15, 2026 14:43
@github-actions

Copy link
Copy Markdown

All conflicts have been resolved. Assigned reviewers can now start or resume their review.

@mvadari
mvadari marked this pull request as ready for review July 15, 2026 18:20
@mvadari mvadari removed the DraftRunCI Normally CI does not run on draft PRs. This opts in. label Jul 15, 2026
@mvadari
mvadari requested review from oleks-rip and yinyiqian1 July 15, 2026 18:20

@xrplf-ai-reviewer xrplf-ai-reviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No issues.

Review by Claude Sonnet 4.6 · Prompt: V15

BEAST_EXPECT(a.count() == b.count());
BEAST_EXPECT(a < b);
BEAST_EXPECT(std::max(a, b) == b);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

sponsored == maxU32 tests missed


auto const a = makeCounts(3, 1, 2);
auto const& self = a;
BEAST_EXPECT(a == self); // self-compare

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

a == a

sb2.adjustOwnerCountHook(alice, OwnerCounts(), counts);

// The parent has no entry for alice yet
BEAST_EXPECT(sb.ownerCountHook(alice, OwnerCounts()) == OwnerCounts());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

aslo sb2 should be tested here

sb.adjustOwnerCountHook(bob, OwnerCounts(), higher);
{
PaymentSandbox sb2(&sb);
sb2.adjustOwnerCountHook(bob, OwnerCounts(), lower);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

test sb2 here too

// fields (Sequence, Fee, ...) must be fixed before the sponsor
// signs; SponsorSignature and Signers are not signing fields, so
// later sign_for calls don't invalidate the sponsor's signature.
auto setupTx = [&]() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please create accountSet helper, like fset(), and use it more standard way, like
env(accountSet(..), Fee(..), MSIg(...))

resp.toStyledString());
}
{
// Signed SponsorSignature (non-empty TxnSignature)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same here - why to use direct json editing, when we have a lot of helpers?

};

json::Value tx;
tx[jss::Account] = alice.human();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same here - please use UT helpers (Fee(..), SIg(..) Sponsor...)


// Sponsorship with a reserve budget where prefunded (unsigned) use
// of the reserve budget requires a sponsor signature.
env(sponsor::set_reserve(sponsor, tfSponsorshipSetRequireSignForReserve, 10),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Named not according to standard, should be setReserve

// (lsfSponsorshipRequireSignForFee), and a co-signed transaction
// still draws the fee from the prefunded FeeAmount because
// getFeePayer prefers the Sponsorship object when it exists.
env(sponsor::set_fee(sponsor, tfSponsorshipSetRequireSignForFee, XRP(10)),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same - bad name, should be setFee

// it before the Oracle-specific preflight checks run.
{
json::Value oracleSet;
oracleSet[jss::TransactionType] = jss::OracleSet;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Use Oracle helpers, not direct json editing

Comment thread src/test/app/TxQ_test.cpp
checkMetrics(*this, env, 0, 6, 5, 3);
}

void

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

please add tests for N transactions with the same sponsor and where N-2-ish transaction's fee exhaust sponsor balance

Ter(temBAD_REGKEY));
env.close();
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Missed batch tests which exhaust sponsor / sponsorship balance through fee and reserve (6 txs and sponsor balance depleted on 3d or 4th)


auto tx = noop(alice);
tx[sfSponsor.jsonName] = sponsor.human();
tx[sfSponsorFlags.jsonName] = static_cast<std::uint32_t>(spfSponsorFee);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

sponsor::As

}

void
testFeeOnlySponsorshipObjectRouting()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

maybe put it next to testCosignedTransferConsumesPrefundedBudget

BEAST_EXPECT(env.balance(alice) == target);
BEAST_EXPECT(env.balance(bob) == target);

// Control: for the unsponsored account, xrpLiquid = balance - base

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please call xrplLiquid directly here
also call accountReserve and checkReserve here or in similar test

}

void
testThirdPartyEscrowCancelRefundsSponsor()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

missed similar test where expired sponsored credentials were deleted indirectly.

MPTTester mptt(env, alice, {.fund = false});
mptt.create({.flags = tfMPTCanLock});
json::Value jvSet;
jvSet[sfTransactionType.jsonName] = jss::MPTokenIssuanceSet;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why not use MPTSet structure for creating MPTokenIssuanceSet with MPTTester

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants