Skip to content

feat: add finalized reward allocation ledger - #422

Open
olatundefay-prog wants to merge 2 commits into
DigiNodes:mainfrom
olatundefay-prog:reward
Open

olatundefay-prog wants to merge 2 commits into
DigiNodes:mainfrom
olatundefay-prog:reward

Conversation

@olatundefay-prog

Copy link
Copy Markdown

Closes #370

V2-SC-019 — Implement Reward Allocation Ledger

Copy link
Copy Markdown
Contributor

@olatundefay-prog review of head 27757d7c1239b871c74e719020372c87368c96a6: assignment and dependencies are valid, but this protocol-critical rewards ledger is not mergeable yet.

  • fund() records the requested amount without checking the allocator's balance delta. A fee-on-transfer or otherwise non-standard ERC-20 can make _funded and claimable entitlements exceed actual custody. Use exact received-amount accounting (or explicitly reject transfer mismatches) and add regression coverage.
  • Funds can be stranded: fund() remains callable after settlementId is finalized; zero allocations are accepted; partial allocation leaves an unrecoverable remainder; and multiple assets may be funded under one settlement although _finalized[settlementId] permits finalizing only one asset. Define the canonical single-/multi-asset model and ensure every funded unit is allocated, explicitly retained/refunded, or recoverable without cross-settlement consumption.
  • The allocator accepts caller-supplied outcome, categories, weights, and totals without enforcing outcome/category compatibility or binding them to the frozen final settlement record. Integrate the narrow canonical settlement/final-resolution interface so disputed, undisputed, conclusive, and inconclusive paths cannot produce contradictory allocations.
  • supportsInterface() omits type(IV2Module).interfaceId, despite the V2 module contract requiring it. Add the explicit interface response and conformance test.
  • The deployment/documentation is contradictory: it says to register the allocator under SETTLEMENT, while _onlySettlementModule() expects the registered SETTLEMENT implementation to call the allocator. Registering the allocator there would replace the settlement engine and make external funding/finalization impossible. Give the ledger its canonical module identity, preserve SETTLEMENT as the authorized caller, and add deployment/composition coverage.
  • Required verification is missing: the PR has only three Hardhat tests and no complete conclusive true/false, inconclusive, disputed/undisputed, category-compatibility, rounding/overflow boundary, duplicate-category, insufficient-claimable, exact-custody, accounting-invariant fuzz, or settlement-integration coverage. Use Math.mulDiv or prove multiplication bounds for amount * weight.

The exact-head CI, Fuzz Testing, and Gas Regression Detection runs are action_required; this is maintainer workflow authorization, not a contributor failure. After the fixes, run npm run compile && npm test && npm run compile:forge && forge test -vv && npm run test:gas, then request authorized fresh Actions and an independent human maintainer approval on the exact SHA.

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.

V2-SC-019 — Implement Reward Allocation Ledger

2 participants