Conversation
… (C-2) The buyback hook specification was hardcoded to index [1], but when usesTiered721Hook=false the array is only size 1 (valid index: [0]). This caused an out-of-bounds revert on every payment to non-721 revnets when the buyback hook activated. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
newBorrowAmount and newCollateralCount are uint256 but stored as uint112. Without overflow checks, values exceeding uint112.max are silently truncated, enabling fund theft by borrowing the full amount but only recording the truncated (tiny) loan. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Prevent no-op repayments where both repayBorrowAmount and collateralCountToReturn are zero, which would burn and re-mint loan NFTs without any actual repayment, allowing unlimited loan NFT inflation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…dedWith Add DIRECTORY.isTerminalOf check to prevent unauthorized callers from invoking afterCashOutRecordedWith with crafted parameters. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Prevent cross-source collateral reallocation that could extract value when different terminal/token pairs have different surplus ratios. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The _borrowableAmountFrom function reads live surplus, raising concerns about flash loan manipulation via addToBalanceOf. Analysis shows this attack is economically irrational: addToBalanceOf permanently donates funds, and the extra borrowable amount is always a fraction of the donation (bounded by collateralCount/totalSupply < 1). The bonding curve's concavity with non-zero cashOutTaxRate makes it even worse for the attacker. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add NatSpec documenting that project conversion does not account for pre-existing token supply and that existing holders should be informed of the economic implications. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add comprehensive NatSpec documenting that expired loan liquidation permanently destroys collateral, and that borrowers have the full loan duration to repay. This is intentional design to keep the protocol simple. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove per-revnet loans config param in favor of a single immutable LOANS address set at REVDeployer construction time. All revnets now share the same loans contract. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The M-27 fix rejects repayLoan calls where both repay amount and collateral return are zero. Add vm.assume to skip those fuzz inputs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Instead of granting USE_ALLOWANCE per-revnet on every deploy, grant it once in the constructor with projectId=0 (wildcard). Saves gas on each deploy. Adds integration test verifying the permission. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
REVLoans constructor now takes IJBController directly instead of IREVDeployer. Removes the REVNETS check so any project can use loans at their own risk. Deploy script no longer needs the address(0) placeholder + re-computation dance. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Loan fund access limits were redundantly specified in REVConfig.loanSources when all the same information was already available in terminalConfigurations. Now _makeLoanFundAccessLimits iterates over each terminal's accounting contexts directly, creating one surplus allowance per terminal+token pair. - Remove REVLoanSource[] loanSources from REVConfig struct - Rewrite _makeLoanFundAccessLimits to derive from terminalConfigurations - Delete _matchingCurrencyOf (no longer needed) - Remove REVDeployer_LoanSourceDoesntMatchTerminalConfigurations error - Update Deploy.s.sol and all 12 test files Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When the REV fee payment fails (try-catch), zero out revFeeAmount so the borrower receives the full amount instead of it being stuck in the contract. For ERC-20 tokens, decrease the dangling allowance. Fixes the dead-code ternary (L-23) as well. This keeps the try-catch by design — a revnet should never be blocked by fee infrastructure issues. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Permissionless function to burn project tokens stuck in REVDeployer from reserved token distribution when splits don't sum to 100%. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
A non-terminal caller would just be donating their own funds as fees. There's nothing to exploit — no caller validation required. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace per-revnet buybackHookOf mapping with immutable BUYBACK_HOOK_REGISTRY. Auto-configure buyback pools during deployment using sensible defaults (1% fee tier, 30min TWAP window) read from terminal accounting contexts. Fix C-2 array OOB in beforePayRecordedWith hook specification indexing. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Split operator can now manage swap terminal selection in the registry, alongside existing buyback pool and TWAP configuration. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…payable removal - Normalize token amounts by decimal difference in _totalBorrowedFrom before aggregating across different token decimals - Change liquidation threshold from > to >= for LOAN_LIQUIDATION_DURATION - Remove payable from reallocateCollateralFromLoan and reject msg.value since the function only moves existing collateral Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Fixes collateral→collateralCount and return value name mismatches in borrowableAmountFrom, borrowFrom, repayLoan, and reallocateCollateralFromLoan. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Updates 3 calls to currentReclaimableSurplusOf in REVLoansSourced.t.sol to use the renamed parameter cashOutCount instead of tokenCount. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Update all import paths, dependency references, and prose from Juicebox V5 to Juicebox V6. Repo folder name headers preserved as-is. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Matching hash covers economic parameters but not terminal/accounting configs. Revnets on non-ETH-native chains must use WETH/USDC ERC20 accounting instead of NATIVE_TOKEN to avoid asset mismatch. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Documents the risk of using NATIVE_TOKEN as a terminal accounting context on non-ETH-native chains, and that the matching hash does not cover terminal configurations. References INTEROP-6 and SECURITY.md. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Foundry's correct key is evm_version. vm_version is silently ignored, meaning EVM version was not being enforced. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Use IJBRulesetDataHook instead of IJBBuybackHookRegistry so the REVDeployer has no notion of a registry — just a generic data hook. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This change belongs in a separate PR with dedicated tests. The fee recovery logic (catch block with safeDecreaseAllowance and revFeeAmount zeroing) will be submitted independently. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When feeTerminal.pay() reverts, the borrower now receives the REV fee amount back instead of losing it. For ERC-20 tokens, the dangling allowance to the fee terminal is cleaned up via safeDecreaseAllowance. Changes: - Remove redundant revFeeAmount == 0 ternary (already inside if > 0) - Add catch block: decrease ERC-20 allowance and zero out revFeeAmount - Fix comments: "beneficiary" → "fee terminal", "msg.sender" → "beneficiary" 9 new tests covering native ETH, ERC-20, fuzz, and no-stuck-funds scenarios. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
fix: align IREVLoans argument names with implementation
Add verified README.md and SKILLS.md
…ing-risk Document cross-chain NATIVE_TOKEN accounting mismatch risk (INTEROP-6)
Append V6 to deploy salts
Fix vm_version typo to evm_version in foundry.toml
L-15: Document stage transition impact on outstanding loans L-16: Document intentional 100% LTV with cashOutTaxRate buffer L-24: Clear loan storage after NFT burn for gas refund Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Two bugs caused by _repayLoan deleting loan storage: 1. repayLoan reads loan.source.token after delete → address(0) → SafeERC20 fails. Fix: cache sourceToken before _repayLoan. 2. _repayLoan returns storage pointer after delete → all zeros. Fix: snapshot loan to memory before delete in full-repayment path. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…eanup) Verifies stage transitions reduce borrowable amount (higher cashOutTaxRate), loan storage is cleared after full repayment and after liquidation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Adds 3 tests for PR #11 gaps: - Partial repay clears old loan storage (else branch in _repayLoan) - Excess ETH refund uses cached sourceToken (use-after-delete regression) - Reallocation clears old loan storage after replacement Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Fix/low findings revnet
Snapshot deltas from current loan state, write loan.amount and loan.collateral BEFORE any external calls, then interact using pre-computed deltas. Any reentrant call sees updated state. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- ReentrantBorrower contract reads loan state during ETH callback, verifies CEI pattern exposes finalized (not stale) state - Atomic consistency test: loan.amount == totalBorrowedFrom after each borrow and both == 0 after full repay - Rapid borrow/repay sequence: 3 cycles with clean accounting - Source verification documented: all 4 _adjust helpers use memory copies and delta parameters, never read loan.amount/loan.collateral Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
fix/c3 CEI pattern
Three new tests exercising _totalBorrowedFrom decimal normalization: 1. Zero-borrow continue path (TOKEN source has 0 borrowed → skipped) 2. TOKEN (6-dec) source borrow normalized correctly in 18-dec ETH context 3. Both ETH and TOKEN sources with cross-decimal aggregation Also documents price feed precision limitation: inverse price at 6 decimals truncates to 0 (mulDiv(1e6,1e6,1e21)=0), requiring TOKEN borrows before ETH. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…Spec - _totalBorrowedFrom: explain decimal normalization requirement and inverse price feed precision limitation at low decimal counts - reallocateCollateralFromLoan: explain why function is not payable Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
fix: REVLoans mixed-decimal normalization, liquidation boundary, and …
Fix fee payment error recovery in REVLoans
Resolve conflict in src/REVLoans.sol by keeping both the source mismatch validation (PR #13) and the not-payable NatSpec comment (from version/6). Fix pre-existing test issues: REVDeployer constructor args and currentReclaimableSurplusOf parameter name (cashOutCount → tokenCount). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Fix/m10 cross source reallocation
- Merge version/6 into hardcode-loans-address (auto-merged, no source conflicts) - Update 14 test files for PR #25 changes: - REVLoans constructor: revnets → controller - REVDeployer constructor: add address(LOANS_CONTRACT) param - REVConfig: remove loanSources and loans fields - Deploy order: REVLoans before REVDeployer - Fix pre-existing test issues: - REVLoansSourced: cashOutCount → tokenCount param name - REVLoansFeeRecovery: add missing buybackHookConfiguration param Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…st failures - Resolve merge conflicts with version/6 (keep SourceMismatch error, remove RevnetsMismatch) - Add explicit IJBProjects parameter to REVLoans constructor instead of double indirection - Fix 3 fuzz test boundary failures (liquidation now triggers at >= LOAN_LIQUIDATION_DURATION) - Fix TestPR10 assertion for loan data deletion after liquidation (delete _loanOf[loanId]) - Update deploy script and all 23 test files with new constructor signature - All 144 tests pass Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Hardcode loans address
…ress) - REVDeployer constructor now takes both buybackHook and loans params - hasMintPermissionFor uses both global LOANS and BUYBACK_HOOK immutables - Remove per-revnet buyback setup (now global) and per-revnet loans setup (now global) - Delete REVBuybackHookConfig and REVBuybackPoolConfig structs - Remove buybackHookConfiguration from all deployFor calls across 14 test files - Add MockBuybackDataHook for tests (BUYBACK_HOOK can no longer be address(0)) - Update TestPR22 assertion: buyback hook spec is always present (1, not 0) - All 144 tests pass Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Hardcode buyback hook registry
REVDeployer fits with 43 bytes margin. Better gas per call. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
chore: v6 QC pass
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.
Description
What does this PR: do, how, why?
Limitations & risks
Are there any trade-off or new vulnarbility surface based on theses changes?
Check-list
Interactions
These changes will impact the following contracts:
Directly:
Indirectly: