Skip to content

fix: address comments #129

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
0xChin merged 2 commits into from
Jan 15, 2026
Merged

fix: address comments #129

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
e1519f9
refactor: inlined if-skip
0xChin Jan 15, 2026
e01ac88
chore: improve comments in IntegrationBase
0xChin Jan 15, 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
16 changes: 12 additions & 4 deletions test/integration/IntegrationBase.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -369,18 +369,23 @@ abstract contract IntegrationBase is Test {
/// @param _chainConfig Configuration for the chain being tested (L2 -> L1 withdrawal)
/// @param _opConfig Configuration for OP chain where FeesDepositor forwards funds (L1 -> OP L2)
function _executeDisburseAndAssertWithdrawal(ChainConfig memory _chainConfig, OPConfig memory _opConfig) internal {
// ==================== Step 1: Init withdrawal (Rev Share L2) ====================
// Disburse fees on the Rev Share L2 chain, which triggers L1Withdrawer to initiate a withdrawal to L1
vm.selectFork(_chainConfig.l2ForkId);
vm.warp(block.timestamp + IFeeSplitter(FEE_SPLITTER).feeDisbursementInterval() + 1);

vm.expectEmit(true, true, true, true, _chainConfig.l1Withdrawer);
emit WithdrawalInitiated(_chainConfig.l1WithdrawalRecipient, _chainConfig.expectedWithdrawalAmount);
IFeeSplitter(FEE_SPLITTER).disburseFees();

// Relay the withdrawal message to L1
// ==================== Step 2: Relay withdrawal (L1) ====================
// Relay the L2->L1 withdrawal message. If amount >= threshold, FeesDepositor will initiate a deposit to OP.
vm.selectFork(_chainConfig.l1ForkId);

if (_chainConfig.expectedWithdrawalAmount >= FEES_DEPOSITOR_THRESHOLD) {
// Expect TransactionDeposited event from OP Portal
// ==================== Step 3: Init deposit to OP (L1) ====================
// When withdrawal is relayed on L1, FeesDepositor receives the ETH and initiates a deposit to OP L2.
// Expect TransactionDeposited event from OP Portal.
// Note: FeesDepositor calls L1CrossDomainMessenger.sendMessage(), which calls OptimismPortal.depositTransaction()
// The 'from' address in TransactionDeposited is the aliased L1CrossDomainMessenger (not the FeesDepositor)
vm.expectEmit(true, true, true, false, _opConfig.opPortal);
Expand All @@ -391,6 +396,7 @@ abstract contract IntegrationBase is Test {
""
);

// Relay the withdrawal message on L1 (triggers FeesDepositor to initiate deposit to OP)
_relayL2ToL1Message(
_chainConfig.portal,
_chainConfig.l1Messenger,
Expand All @@ -401,7 +407,8 @@ abstract contract IntegrationBase is Test {
"" // data (empty for ETH transfer)
);

// Now relay the deposit from L1 to OP L2
// ==================== Step 4: Relay deposit (Optimism L2) ====================
// Relay the L1->L2 deposit message on Optimism, delivering ETH to the final recipient
vm.selectFork(_opConfig.opL2ForkId);

uint256 recipientBalanceBefore = _opConfig.feesDepositorTarget.balance;
Expand All @@ -424,7 +431,8 @@ abstract contract IntegrationBase is Test {
"FeesDepositor target should receive the withdrawal amount"
);
} else {
// FeesDepositor holds the ETH (below threshold)
// Below threshold: FeesDepositor holds the ETH on L1 (no deposit to OP)
// Simply relay the withdrawal to the L1 recipient
uint256 recipientBalanceBefore = _chainConfig.l1WithdrawalRecipient.balance;

_relayL2ToL1Message(
Expand Down
19 changes: 9 additions & 10 deletions test/integration/RevSharePostTaskAssertions.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,14 +64,6 @@ contract RevSharePostTaskAssertionsTest is IntegrationBase {
// Flag to track if env vars are set
bool internal _isEnabled;

/// @notice Modifier to skip tests if required env vars are not set
modifier onlyIfEnabled() {
if (!_isEnabled) {
vm.skip(true);
}
_;
}

function setUp() public {
// Read env vars with defaults to detect if they're set
string memory rpcUrl = vm.envOr("RPC_URL", string(""));
Expand Down Expand Up @@ -112,7 +104,10 @@ contract RevSharePostTaskAssertionsTest is IntegrationBase {
}

/// @notice Assert the Rev Share contract state on the L2 chain
function test_assertRevShareState() public onlyIfEnabled {
function test_assertRevShareState() public {
if (!_isEnabled) {
vm.skip(true);
}
vm.selectFork(_l2ForkId);

_assertL2State(
Expand All @@ -129,7 +124,11 @@ contract RevSharePostTaskAssertionsTest is IntegrationBase {
// Fund vaults so that:
// - First disburse: share < minWithdrawalAmount (below threshold, no withdrawal)
// - Second disburse: total >= minWithdrawalAmount (triggers withdrawal)
function test_withdrawalFlow() public onlyIfEnabled {
function test_withdrawalFlow() public {
if (!_isEnabled) {
vm.skip(true);
}

// ==================== PART 1: Below threshold - no withdrawal ====================
vm.selectFork(_l2ForkId);

Expand Down