Skip to content

test(drive-abci): fix direct-selling tests broken by empty-schedule validation#3876

Merged
QuantumExplorer merged 1 commit into
v3.1-devfrom
test/fix-empty-set-prices-direct-selling-tests
Jun 13, 2026
Merged

test(drive-abci): fix direct-selling tests broken by empty-schedule validation#3876
QuantumExplorer merged 1 commit into
v3.1-devfrom
test/fix-empty-set-prices-direct-selling-tests

Conversation

@QuantumExplorer

@QuantumExplorer QuantumExplorer commented Jun 12, 2026

Copy link
Copy Markdown
Member

Issue being fixed or feature implemented

CI on v3.1-dev is red: two drive-abci direct-selling tests fail with TokenPricingScheduleEmptyError where they expect SuccessfulExecution (e.g. this run).

This is a semantic conflict between two independently merged PRs:

Each PR was green on its own; together the #3856 tests can no longer plant their fixture.

What was done?

Test-only changes in batch/tests/token/direct_selling/mod.rs — no production code touched:

  • Split the create_token_with_pricing helper: a new create_token_with_pricing_result returns the set-price processing result, while the existing helper keeps asserting success on top of it.
  • test_direct_purchase_empty_set_prices_does_not_panic now covers both layers of the defense:
    1. asserts that setting an empty SetPrices schedule via a state transition is rejected with UnpaidConsensusError(TokenPricingScheduleEmptyError) (the fix(dpp): reject empty token pricing schedules to prevent a direct-purchase chain halt #3865 check, end-to-end);
    2. plants the empty schedule directly through Drive::token_set_direct_purchase_price (bypassing validation, simulating state written before the check existed) and asserts a purchase is still rejected gracefully as TokenNotForDirectSale with no panic and no token credited (the fix(drive): return error instead of panicking on empty SetPrices direct purchase #3856 transformer fix).
  • test_successful_direct_purchase_multiple_tokens uses a non-empty single-tier SetPrices schedule for its third token instead of the no-longer-storable empty one, keeping three distinct schedule shapes for the price fetch/proof round-trip.

How Has This Been Tested?

cargo test -p drive-abci --all-features token_selling_tests — all 10 tests pass, including the two that were failing in CI. Also verified these are the only two places in the workspace (outside the #3865 validator's own unit tests) that construct an empty SetPrices schedule.

Breaking Changes

None.

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have added "!" to the title and described breaking changes in the corresponding section if my code contains any
  • I have made corresponding changes to the documentation if needed

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Improved validation and error handling for empty token pricing schedules to prevent system crashes when processing invalid pricing configurations.
  • Tests

    • Enhanced regression tests to verify graceful handling of malformed pricing data and ensure proper error rejection during token transactions.

…alidation

PRs #3856 and #3865 merged independently and conflict semantically:
#3856 added tests that plant an empty SetPrices schedule via a state
transition, while #3865 now rejects that transition at structure
validation with TokenPricingScheduleEmptyError.

Update the tests to assert the new upfront rejection, plant the empty
schedule directly through the drive API to keep the transformer
defense-in-depth regression coverage, and use a non-empty tiered
schedule in the multi-token pricing test.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 86d7340b-e540-4fa1-8cb6-dfc06dd349c1

📥 Commits

Reviewing files that changed from the base of the PR and between 01e1ae2 and 04b0755.

📒 Files selected for processing (1)
  • packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/direct_selling/mod.rs

📝 Walkthrough

Walkthrough

This PR refactors token direct-selling tests to validate SetPrices schedule handling. A test helper is refactored to expose state transition results, test data is corrected to use valid pricing, and a regression test is rewritten to assert empty schedule rejection and verify downstream defensive error handling.

Changes

Token SetPrices Validation Tests

Layer / File(s) Summary
Test helper refactoring for processing results
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/direct_selling/mod.rs
Added import for StateTransitionsProcessingResult; refactored create_token_with_pricing to delegate to a new create_token_with_pricing_result helper that returns (DataContract, Identifier, StateTransitionsProcessingResult) instead of only asserting success internally.
Multiple tokens test data update
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/direct_selling/mod.rs
Replaced the empty tiered pricing schedule with a non-empty one in the "successful direct purchase multiple tokens" test data.
Empty SetPrices regression test rewrite
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/direct_selling/mod.rs
Updated test_direct_purchase_empty_set_prices_does_not_panic to assert that empty SetPrices schedules are rejected during structure validation with TokenPricingScheduleEmptyError, then plant the empty schedule directly into state via drive API to simulate pre-validation state, and verify that subsequent purchase attempts are rejected gracefully without panicking.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • dashpay/platform#3856: Targets the same empty TokenPricingSchedule::SetPrices edge case and modifies the same regression test with updated error expectations.
  • dashpay/platform#3865: Implements TokenPricingScheduleEmptyError to reject empty TokenPricingSchedule::SetPrices during structure validation, which this PR's tests validate.

Suggested reviewers

  • thepastaclaw
  • shumkov

Poem

🐰 A token's price must never be empty, dear,
So we test the error path loud and clear—
Empty SetPrices caught at the gate,
No panics to fear, all handled straight! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the main change: fixing direct-selling tests that were broken by the addition of empty-schedule validation. It directly relates to the changeset which updates tests to handle the semantic conflict between two merged PRs.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch test/fix-empty-set-prices-direct-selling-tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@thepastaclaw

thepastaclaw commented Jun 12, 2026

Copy link
Copy Markdown
Collaborator

✅ Review complete (commit 04b0755)

@thepastaclaw thepastaclaw left a comment

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.

Code Review

Test-only PR that aligns with the new empty-SetPrices structure validation (#3865) and preserves the transformer defense-in-depth regression by planting the empty schedule directly through the drive API. Scope is minimal and correct. The agent findings either describe pre-existing test gaps unrelated to this PR's goal, or benign nitpicks not worth blocking on.

@codecov

codecov Bot commented Jun 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.22%. Comparing base (e2cde1d) to head (04b0755).
⚠️ Report is 2 commits behind head on v3.1-dev.

Additional details and impacted files
@@              Coverage Diff              @@
##           v3.1-dev    #3876       +/-   ##
=============================================
+ Coverage     70.82%   87.22%   +16.39%     
=============================================
  Files            20     2641     +2621     
  Lines          2797   328569   +325772     
=============================================
+ Hits           1981   286597   +284616     
- Misses          816    41972    +41156     
Components Coverage Δ
dpp 87.69% <ø> (∅)
drive 86.14% <ø> (∅)
drive-abci 89.45% <ø> (∅)
sdk ∅ <ø> (∅)
dapi-client ∅ <ø> (∅)
platform-version ∅ <ø> (∅)
platform-value 92.20% <ø> (∅)
platform-wallet ∅ <ø> (∅)
drive-proof-verifier 49.55% <ø> (∅)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@QuantumExplorer
QuantumExplorer merged commit 29afa55 into v3.1-dev Jun 13, 2026
22 checks passed
@QuantumExplorer
QuantumExplorer deleted the test/fix-empty-set-prices-direct-selling-tests branch June 13, 2026 00:08
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