fix: transition milestones to Paid, paginate funder reports, surface … - #842
Merged
Conversation
…index cap, wire notifications (PhasoraLabs#696 PhasoraLabs#697 PhasoraLabs#698 PhasoraLabs#699) - PhasoraLabs#696: transition milestones to Paid once their real payout confirms (finalize_grant_release / execute_escrow_release), instead of leaving them stuck at Approved forever. portfolio::earnings_by_token and data_export's paid-out totals only ever counted Paid milestones, so every normally-paid contributor showed zero earnings. - PhasoraLabs#697: funder_report's get_report/token_summary/dashboard_summary no longer hard-code a 50-grant window; they now aggregate every grant a funder has contributed to. - PhasoraLabs#698: grant_index::push_to_index emits an IndexCapReached event instead of silently dropping entries once an index hits MAX_INDEX_ENTRIES, so the condition is observable instead of silent data loss. - PhasoraLabs#699: wire notification::emit_notification into grant_create, milestone_submit, milestone_vote (approve/reject), and dispute_raise so subscribers actually receive NewGrant/MilestoneSubmitted/ MilestoneApproved/MilestoneRejected/DisputeRaised notifications. Also adds the ContractError::TooManyPublicReviews variant referenced by open_review.rs — without it the crate fails to compile at all, which was breaking `cargo clippy --workspace --lib --target wasm32v1-none` (the exact command CI runs) even before this branch's changes.
|
@Favour4712 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
…-699-milestone-earnings-pagination-index-notifications # Conflicts: # contracts/contracts/stellar-grants/src/errors.rs # contracts/contracts/stellar-grants/src/lib.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes four correctness/data-integrity bugs in the
stellar-grantscontract:finalize_grant_release/execute_escrow_releaseinlib.rs) only ever left milestones atMilestoneState::Approved— nothing transitioned them toPaid. Sinceportfolio::earnings_by_tokenanddata_export's paid-out totals only countPaidmilestones, every contributor paid through the normal flow showed zero earnings, and every fully-paid grant reportedpaid_out: 0in exports.funder_reporthardcoded 50-grant window:get_report,token_summary,total_in_escrow, anddashboard_summaryall calledgrant_summaries(env, funder, 0, 50)with a hardcoded cap. Any funder with more than 50 grants silently had everything past the 50th grant omitted from their report/dashboard.grant_indexsilently dropping entries past the 10,000-item cap:push_to_indexreturned()and no-opped once an index hitMAX_INDEX_ENTRIES, so grants beyond the cap became permanently invisible toby_owner/by_status/by_token/recent/data_exportwith zero observability.notification::emit_notificationexisted but was never called from anywhere in the crate, so subscribers never actually received a notification for any grant/milestone/dispute lifecycle event.Changes
#696 —
contracts/contracts/stellar-grants/src/governance.rs,lib.rs,data_export.rsgovernance::mark_milestones_paid(env, grant_id, total_milestones): transitions every still-Approvedmilestone toPaidand emits the (previously-defined-but-never-called)MilestonePaidevent.finalize_grant_releaseright beforecomplete_grant(immediate-release path) and fromexecute_escrow_releaseright after a multisig-gated release actually executes — not from inside the payout loop itself, so a milestone is never markedPaiduntil its fund transfer has actually confirmed (a multisig-pending release correctly leaves it atApproveduntil execution).compute_total_paid_if_quorum_readynow accepts milestones already inPaidstate (not justApproved), since it's re-run a second time fromexecute_escrow_releaseafter some milestones may already have been marked paid.data_export'sapproved_atfield now also considersPaidmilestones (previously it reportedNoneforapproved_atthe moment a milestone moved pastApproved).#697 —
contracts/contracts/stellar-grants/src/funder_report.rsall_grant_summarieshelper that fetches every grant for a funder (no hardcoded cap), used byget_report,token_summary, anddashboard_summary.total_in_escrowis fixed transitively since it delegates totoken_summary.#698 —
contracts/contracts/stellar-grants/src/grant_index.rs,events.rspush_to_indexnow takes an explicitcapand returns whether the id ended up in the list; on a cap hit it emits a newIndexCapReachedevent instead of silently no-opping.#699 —
contracts/contracts/stellar-grants/src/lib.rsnotification::emit_notificationis now called aftergrant_create(NewGrant, scopedPerContributor(owner)), after milestone submission (MilestoneSubmitted, scopedPerGrant), inside themilestone_voteapproved/rejected branches (MilestoneApproved/MilestoneRejected, scopedPerGrant), and afterdispute_raise(DisputeRaised, scopedPerGrant).Build-blocker (pre-existing, unrelated to the four issues above)
contracts/contracts/stellar-grants/src/errors.rs: added the missingContractError::TooManyPublicReviewsvariant.open_review.rsalready referenced it, but it was never added to the enum — this failscargo clippy --workspace --lib --target wasm32v1-none -- -D warnings(the exact command this repo's CI runs) onmaintoday, independent of this PR. Left everything else about that module untouched.Tests
tests/test_milestone_paid_earnings.rs— fullgrant_create→grant_fund→milestone_submit→milestone_vote→grant_completeflow; asserts the milestone ends upPaid,portfolio_earnings_by_tokenreflects the payout (and is zero beforehand),export_grants'spaid_outreflects it, and themilestone_paidevent fires.tests/test_notification_emission.rs— three tests coveringNewGrant,MilestoneSubmitted/MilestoneApproved, andDisputeRaised: subscribe, trigger the action, assert anotificationevent was published (andget_subscribersreturns the subscriber).funder_report.rsunit test seeds 55 grants for one funder and assertsget_report,dashboard_summary, andtoken_summaryall reflect the full 55, not just 50.grant_index.rsunit tests use a reduced cap (3) to demonstratepush_to_indexnow reports failure and emitsIndexCapReachedinstead of silently dropping an entry, and that re-pushing an existing id at a full cap is still a safe no-op.CI
This repo's CI (
.github/workflows/ci.yml,contractsjob) runs exactly:It does not run
cargo test. All three commands were run locally against this branch and pass cleanly.cargo testwas also run locally for the new integration tests (tests/test_milestone_paid_earnings.rs,tests/test_notification_emission.rs) and they pass. The crate's internal#[cfg(test)]unit-test target (cargo test --lib) currently fails to compile onmaindue to extensive pre-existing, unrelated breakage in files this PR doesn't touch (milestone_extension.rs,referral.rs,merkle.rs,split_payment.rs,storage/helpers.rs,compliance.rs,lockup.rs— missing trait imports, a missingDefaultderive, a moved-value bug, wrong argument counts). Since CI never invokescargo test, none of this is gated — but it's worth a maintainer's attention separately, and I've intentionally left it out of this PR's scope. The two new unit tests added tofunder_report.rsandgrant_index.rsare verified correct by hand-tracing (and were confirmed to pass in isolation via a temporary local patch of the unrelated breakage, which was not included in this PR).One more related finding surfaced while writing the #696/#699 tests:
milestone_vote(approve=true)currently requires a milestone to already have a fully-satisfied checklist (checklist::all_required_approved) or it panics withRequiredCriteriaNotMet—all_required_approveddefaults tofalsewhen no checklist was ever attached, and checklists are optional at grant creation. In practice this means no milestone can be approved through the normal reviewer-vote flow unless a checklist was explicitly defined, submitted, and reviewed first, even though nothing ingrant_create/milestone_submitrequires one. This reproduces onmaintoo (the existingtest_event_emission_on_milestone_votetest intests/test_event_emission.rsalready fails this way, unrelated to my changes). I routed around it in my new tests by attaching and clearing a trivial optional checklist criterion, but did not fix the underlying gate — it felt out of scope for these four issues and worth a maintainer decision on intended behavior. Flagging it here rather than silently working around it.Closes
Closes #696
Closes #697
Closes #698
Closes #699