Skip to content

fix: address audit findings H-8, H-9, M-11 #42

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mejango merged 2 commits into from
Mar 23, 2026
Merged

fix: address audit findings H-8, H-9, M-11 #42

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bb211bb
fix: address audit findings H-8, H-9, M-11
mejango Mar 23, 2026
c85e33f
style: forge fmt
mejango Mar 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 30 additions & 7 deletions script/Deploy.s.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
contract Deploy is Script, Sphinx {
error Deploy_ExistingAddressMismatch(address expected, address actual);
error Deploy_ProjectIdMismatch(uint256 expected, uint256 actual);
error Deploy_ProjectNotOwned(uint256 projectId);
error Deploy_PriceFeedMismatch(uint256 projectId, uint256 pricingCurrency, uint256 unitCurrency);

// ════════════════════════════════════════════════════════════════════
Expand Down Expand Up @@ -854,12 +855,7 @@ contract Deploy is Script, Sphinx {
// ════════════════════════════════════════════════════════════════════

function _deploySuckers() internal {
_deploySuckersOptimism();
_deploySuckersBase();
_deploySuckersArbitrum();
_deploySuckersCCIP();

// Deploy the registry and pre-approve deployers.
// Deploy the registry FIRST — singleton sucker constructors consume it as an immutable.
(address registry, bool registryDeployed) = _isDeployed(
SUCKER_REGISTRY_SALT,
type(JBSuckerRegistry).creationCode,
Expand All @@ -871,6 +867,13 @@ contract Deploy is Script, Sphinx {
_directory, _permissions, safeAddress(), _trustedForwarder
);

// Deploy singleton implementations and deployers (they reference _suckerRegistry).
_deploySuckersOptimism();
_deploySuckersBase();
_deploySuckersArbitrum();
_deploySuckersCCIP();

// Pre-approve deployers in the registry.
if (_preApprovedSuckerDeployers.length != 0) {
for (uint256 i; i < _preApprovedSuckerDeployers.length; i++) {
if (!_suckerRegistry.suckerDeployerIsAllowed(_preApprovedSuckerDeployers[i])) {
Expand Down Expand Up @@ -1511,6 +1514,12 @@ contract Deploy is Script, Sphinx {
// ════════════════════════════════════════════════════════════════════

function _deployRevnet() internal {
// Skip revnet deployment when the Uniswap stack was not deployed — revnets require buyback + router
// registries.
if (address(_buybackRegistry) == address(0)) {
return;
}

_revProjectId = _ensureProjectExists(_REV_PROJECT_ID);

// Deploy REVLoans.
Expand Down Expand Up @@ -1661,6 +1670,9 @@ contract Deploy is Script, Sphinx {
// ════════════════════════════════════════════════════════════════════

function _deployCpnRevnet() internal {
// Skip when the Uniswap stack was not deployed — CPN revnet requires buyback + router registries.
if (address(_buybackRegistry) == address(0)) return;

address operator = 0x240dc2085caEF779F428dcd103CFD2fB510EdE82;

JBAccountingContext[] memory accountingContexts = new JBAccountingContext[](1);
Expand Down Expand Up @@ -1831,6 +1843,9 @@ contract Deploy is Script, Sphinx {
// ════════════════════════════════════════════════════════════════════

function _deployNanaRevnet() internal {
// Skip when the Uniswap stack was not deployed — NANA revnet requires buyback + router registries.
if (address(_buybackRegistry) == address(0)) return;

uint256 feeProjectId = _FEE_PROJECT_ID;
address operator = 0x80a8F7a4bD75b539CE26937016Df607fdC9ABeb5;

Expand Down Expand Up @@ -1906,6 +1921,9 @@ contract Deploy is Script, Sphinx {
// ════════════════════════════════════════════════════════════════════

function _deployBanny() internal {
// Skip when the Uniswap stack was not deployed — Banny revnet requires buyback + router registries.
if (address(_buybackRegistry) == address(0)) return;

if (_projects.count() >= _BAN_PROJECT_ID && address(_directory.controllerOf(_BAN_PROJECT_ID)) != address(0)) {
return;
}
Expand Down Expand Up @@ -2184,7 +2202,12 @@ contract Deploy is Script, Sphinx {

function _ensureProjectExists(uint256 expectedProjectId) internal returns (uint256) {
uint256 count = _projects.count();
if (count >= expectedProjectId) return expectedProjectId;
if (count >= expectedProjectId) {
if (_projects.ownerOf(expectedProjectId) != safeAddress()) {
revert Deploy_ProjectNotOwned(expectedProjectId);
}
return expectedProjectId;
}

uint256 created = _projects.createFor(safeAddress());
if (created != expectedProjectId) revert Deploy_ProjectIdMismatch(expectedProjectId, created);
Expand Down
141 changes: 66 additions & 75 deletions test/fork/HookCompositionFork.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,16 @@ contract HookCompositionForkTest is EcosystemForkTest {
uint256 cashOutCount = payerTokens / 2;

vm.prank(PAYER);
jbMultiTerminal().cashOutTokensOf({
holder: PAYER,
projectId: revnetId,
cashOutCount: cashOutCount,
tokenToReclaim: JBConstants.NATIVE_TOKEN,
minTokensReclaimed: 0,
beneficiary: payable(PAYER),
metadata: ""
});
jbMultiTerminal()
.cashOutTokensOf({
holder: PAYER,
projectId: revnetId,
cashOutCount: cashOutCount,
tokenToReclaim: JBConstants.NATIVE_TOKEN,
minTokensReclaimed: 0,
beneficiary: payable(PAYER),
metadata: ""
});

// Fee project balance should have increased (2.5% fee on cashout).
uint256 feeBalanceAfter = _terminalBalance(FEE_PROJECT_ID, JBConstants.NATIVE_TOKEN);
Expand All @@ -58,9 +59,7 @@ contract HookCompositionForkTest is EcosystemForkTest {
assertGt(feeAccrued, 0, "fee accrued should be positive");

// Payer should have fewer tokens.
assertEq(
jbTokens().totalBalanceOf(PAYER, revnetId), payerTokens - cashOutCount, "payer tokens should decrease"
);
assertEq(jbTokens().totalBalanceOf(PAYER, revnetId), payerTokens - cashOutCount, "payer tokens should decrease");
}

/// @notice Mock fee terminal to revert on external pay(). Cash out tokens.
Expand All @@ -74,10 +73,7 @@ contract HookCompositionForkTest is EcosystemForkTest {
_buildTwoStageConfigWithLPSplit(7000, 2000, 2000);

(uint256 revnetId,) = REV_DEPLOYER.deployFor({
revnetId: 0,
configuration: cfg,
terminalConfigurations: tc,
suckerDeploymentConfiguration: sdc
revnetId: 0, configuration: cfg, terminalConfigurations: tc, suckerDeploymentConfiguration: sdc
});

// Pay to build surplus.
Expand All @@ -87,15 +83,16 @@ contract HookCompositionForkTest is EcosystemForkTest {
// Do a NORMAL cashout first to measure baseline fee accrual.
uint256 borrowerTokens = jbTokens().totalBalanceOf(BORROWER, revnetId);
vm.prank(BORROWER);
jbMultiTerminal().cashOutTokensOf({
holder: BORROWER,
projectId: revnetId,
cashOutCount: borrowerTokens / 4,
tokenToReclaim: JBConstants.NATIVE_TOKEN,
minTokensReclaimed: 0,
beneficiary: payable(BORROWER),
metadata: ""
});
jbMultiTerminal()
.cashOutTokensOf({
holder: BORROWER,
projectId: revnetId,
cashOutCount: borrowerTokens / 4,
tokenToReclaim: JBConstants.NATIVE_TOKEN,
minTokensReclaimed: 0,
beneficiary: payable(BORROWER),
metadata: ""
});
uint256 feeAfterNormalCashout = _terminalBalance(FEE_PROJECT_ID, JBConstants.NATIVE_TOKEN);
assertGt(feeAfterNormalCashout, 0, "normal cashout should accrue fees");

Expand All @@ -104,9 +101,7 @@ contract HookCompositionForkTest is EcosystemForkTest {
// The terminal's internal _processFee path is unaffected by external mocks.
vm.mockCallRevert(
address(jbMultiTerminal()),
abi.encodeWithSignature(
"pay(uint256,address,uint256,address,uint256,string,bytes)", FEE_PROJECT_ID
),
abi.encodeWithSignature("pay(uint256,address,uint256,address,uint256,string,bytes)", FEE_PROJECT_ID),
"fee terminal reverted"
);

Expand All @@ -117,15 +112,16 @@ contract HookCompositionForkTest is EcosystemForkTest {
uint256 projectBalanceBefore = _terminalBalance(revnetId, JBConstants.NATIVE_TOKEN);

vm.prank(PAYER);
jbMultiTerminal().cashOutTokensOf({
holder: PAYER,
projectId: revnetId,
cashOutCount: cashOutCount,
tokenToReclaim: JBConstants.NATIVE_TOKEN,
minTokensReclaimed: 0,
beneficiary: payable(PAYER),
metadata: ""
});
jbMultiTerminal()
.cashOutTokensOf({
holder: PAYER,
projectId: revnetId,
cashOutCount: cashOutCount,
tokenToReclaim: JBConstants.NATIVE_TOKEN,
minTokensReclaimed: 0,
beneficiary: payable(PAYER),
metadata: ""
});

// Main assertion: cashout succeeded (try-catch worked).
assertGt(PAYER.balance, payerEthBefore, "payer should receive ETH despite fee terminal revert");
Expand Down Expand Up @@ -204,10 +200,7 @@ contract HookCompositionForkTest is EcosystemForkTest {
_buildTwoStageConfigWithLPSplit(7000, 2000, 2000);

(uint256 revnetId,) = REV_DEPLOYER.deployFor({
revnetId: 0,
configuration: cfg,
terminalConfigurations: tc,
suckerDeploymentConfiguration: sdc
revnetId: 0, configuration: cfg, terminalConfigurations: tc, suckerDeploymentConfiguration: sdc
});

uint256 feeBalancePrev = _terminalBalance(FEE_PROJECT_ID, JBConstants.NATIVE_TOKEN);
Expand All @@ -231,15 +224,16 @@ contract HookCompositionForkTest is EcosystemForkTest {
uint256 payerEthBefore = PAYER.balance;

vm.prank(PAYER);
jbMultiTerminal().cashOutTokensOf({
holder: PAYER,
projectId: revnetId,
cashOutCount: cashOutCount,
tokenToReclaim: JBConstants.NATIVE_TOKEN,
minTokensReclaimed: 0,
beneficiary: payable(PAYER),
metadata: ""
});
jbMultiTerminal()
.cashOutTokensOf({
holder: PAYER,
projectId: revnetId,
cashOutCount: cashOutCount,
tokenToReclaim: JBConstants.NATIVE_TOKEN,
minTokensReclaimed: 0,
beneficiary: payable(PAYER),
metadata: ""
});

assertGt(PAYER.balance, payerEthBefore, "inv: payer received ETH from cashout");

Expand All @@ -260,15 +254,16 @@ contract HookCompositionForkTest is EcosystemForkTest {
if (borrowerTokens > 0) {
uint256 borrowerEthBefore = BORROWER.balance;
vm.prank(BORROWER);
jbMultiTerminal().cashOutTokensOf({
holder: BORROWER,
projectId: revnetId,
cashOutCount: borrowerTokens / 2,
tokenToReclaim: JBConstants.NATIVE_TOKEN,
minTokensReclaimed: 0,
beneficiary: payable(BORROWER),
metadata: ""
});
jbMultiTerminal()
.cashOutTokensOf({
holder: BORROWER,
projectId: revnetId,
cashOutCount: borrowerTokens / 2,
tokenToReclaim: JBConstants.NATIVE_TOKEN,
minTokensReclaimed: 0,
beneficiary: payable(BORROWER),
metadata: ""
});
assertGt(BORROWER.balance, borrowerEthBefore, "inv: borrower received ETH");

feeBalanceNow = _terminalBalance(FEE_PROJECT_ID, JBConstants.NATIVE_TOKEN);
Expand All @@ -285,9 +280,7 @@ contract HookCompositionForkTest is EcosystemForkTest {
assertGt(tokens, 0, "inv: post-AMM pay should return tokens");

// Invariant: LP split hook position exists (accumulated tokens > 0 from step 2).
assertGt(
LP_SPLIT_HOOK.accumulatedProjectTokens(revnetId), 0, "inv: LP split hook has accumulated tokens"
);
assertGt(LP_SPLIT_HOOK.accumulatedProjectTokens(revnetId), 0, "inv: LP split hook has accumulated tokens");

// Final invariant: fee balance only grew.
feeBalanceNow = _terminalBalance(FEE_PROJECT_ID, JBConstants.NATIVE_TOKEN);
Expand All @@ -304,10 +297,7 @@ contract HookCompositionForkTest is EcosystemForkTest {
_buildTwoStageConfigWithLPSplit(0, 0, 2000);

(uint256 revnetId,) = REV_DEPLOYER.deployFor({
revnetId: 0,
configuration: cfg,
terminalConfigurations: tc,
suckerDeploymentConfiguration: sdc
revnetId: 0, configuration: cfg, terminalConfigurations: tc, suckerDeploymentConfiguration: sdc
});

// Pay to build surplus.
Expand All @@ -323,15 +313,16 @@ contract HookCompositionForkTest is EcosystemForkTest {
uint256 payerEthBefore = PAYER.balance;

vm.prank(PAYER);
jbMultiTerminal().cashOutTokensOf({
holder: PAYER,
projectId: revnetId,
cashOutCount: cashOutCount,
tokenToReclaim: JBConstants.NATIVE_TOKEN,
minTokensReclaimed: 0,
beneficiary: payable(PAYER),
metadata: ""
});
jbMultiTerminal()
.cashOutTokensOf({
holder: PAYER,
projectId: revnetId,
cashOutCount: cashOutCount,
tokenToReclaim: JBConstants.NATIVE_TOKEN,
minTokensReclaimed: 0,
beneficiary: payable(PAYER),
metadata: ""
});

// Payer should receive ETH (full pro-rata with 0% tax).
assertGt(PAYER.balance, payerEthBefore, "should receive ETH from 0% tax cashout");
Expand Down
Loading
Loading