Convert fixed-price payouts to pull credits#60
Merged
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code review is billed via overage credits. To resume reviews, an organization admin can raise the monthly limit at claude.ai/admin-settings/claude-code.
Once credits are available, push a new commit or reopen this pull request to trigger a review.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR implements P0-PAY-003: fixed-price minting now records poster, protocol, and curator-reserve credits in StreamDrops (no push-paying during mint), adds reentrancy protection, credit ledgers, withdrawal APIs and accounting views, extensive tests, and aligned docs/ops updates. ChangesFixed-Price Pull-Payment Credit Accounting
Sequence Diagram(s)sequenceDiagram
participant Poster as Poster/Caller
participant mintDrop as StreamDrops.mintDrop
participant credit as _creditFixedPriceProceeds
participant Ledger as CreditLedger
participant Minter as MinterContract
participant Recipient as Recipient
Poster->>mintDrop: mintDrop(msg.value,...)
mintDrop->>credit: when msg.value > 0, split & credit proceeds
credit->>Ledger: update poster/protocol/curator mappings & totals
credit-->>mintDrop: emit FixedPriceCreditCreated
mintDrop->>Minter: mint token (no ETH)
Minter-->>mintDrop: mint succeeds or reverts (rollback)
Poster->>mintDrop: withdrawFixedPriceCreditTo(recipient)
mintDrop->>Recipient: transfer credit funds
Recipient--xmintDrop: may revert (credit preserved)
mintDrop->>Ledger: emit FixedPriceCreditWithdrawn (if transfer succeeded)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
Author
|
@claude please review this fixed-price payment PR. Focus areas:
|
Contributor
Author
|
@coderabbitai review Please review the latest head |
|
✅ Action performedFull review finished. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/StreamFixedPricePayments.t.sol (1)
402-408: 💤 Low valueNote:
selfdestructis deprecated but acceptable for testing in Solidity 0.8.19.The
FixedPriceForceEthcontract usesselfdestructto force ETH into the target contract for testing surplus handling. Whileselfdestructis deprecated and will be removed in future Solidity versions, it remains valid in 0.8.19 and is a standard pattern for testing forced ETH scenarios. This test helper serves its purpose correctly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/StreamFixedPricePayments.t.sol` around lines 402 - 408, The FixedPriceForceEth test helper uses the deprecated selfdestruct pattern; update the file to explicitly document and allow it by adding a short comment above the FixedPriceForceEth contract (and/or a linter pragma) stating this is intentionally using selfdestruct for forced-ETH testing under Solidity 0.8.19, and if your linter flags selfdestruct, add a local linter disable for that rule around the constructor/force function so the FixedPriceForceEth constructor and force(address payable target) remain unchanged for tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@test/StreamFixedPricePayments.t.sol`:
- Around line 402-408: The FixedPriceForceEth test helper uses the deprecated
selfdestruct pattern; update the file to explicitly document and allow it by
adding a short comment above the FixedPriceForceEth contract (and/or a linter
pragma) stating this is intentionally using selfdestruct for forced-ETH testing
under Solidity 0.8.19, and if your linter flags selfdestruct, add a local linter
disable for that rule around the constructor/force function so the
FixedPriceForceEth constructor and force(address payable target) remain
unchanged for tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 209ccfe3-dc69-4efc-b6ed-89d075b74961
📒 Files selected for processing (9)
docs/adr/0003-payment-accounting.mddocs/known-blockers.mddocs/status.mdops/AUTONOMOUS_RUN.mdops/ROADMAP.mdsmart-contracts/StreamDrops.soltest/README.mdtest/StreamDropsIntegrationCharacterization.t.soltest/StreamFixedPricePayments.t.sol
Contributor
Author
|
Merge decision for PR #60:
Proceeding with merge unless a new actionable review artifact appears before merge. |
This was referenced Jun 10, 2026
This was referenced Jun 12, 2026
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
1 participant
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
Converts fixed-price
StreamDropsmint payments from synchronous ETH pushes into ADR 0003 pull-payment accounting.Closes #27.
Roadmap
Security And Maturity
Review Routing
Validation
Local commands run:
make checkor platform equivalent passed.Impact
test/StreamFixedPricePayments.t.soland converts fixed-price integration characterization away from unsafe push-payment expectations.Reviewer Notes
Please focus on the fixed-price accounting boundary: poster/protocol credits are withdrawable now; curator reserve is included in fixed-price owed/surplus accounting but intentionally remains reserved for the later curator-claim workstream rather than ordinary recipient withdrawal.
Summary by CodeRabbit
New Features
Documentation
Tests