Skip to content

Conversation

bxmmm1
Copy link
Collaborator

@bxmmm1 bxmmm1 commented Sep 24, 2025

No description provided.

Copy link

codecov bot commented Sep 24, 2025

Codecov Report

❌ Patch coverage is 73.68421% with 5 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
mainnet-contracts/src/PufferWithdrawalManager.sol 73.68% 5 Missing ⚠️
Files with missing lines Coverage Δ
mainnet-contracts/src/PufferWithdrawalManager.sol 83.58% <73.68%> (-1.64%) ⬇️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@bxmmm1 bxmmm1 requested a review from eladiosch September 24, 2025 11:20
require(batchIndex > $.finalizedWithdrawalBatch, WithdrawalAlreadyFinalized());

uint256 pufETHAmount = withdrawal.pufETHAmount;
address recipient = withdrawal.recipient;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we're using withdrawal.recipient in some checks before this line, we could declare this first and use the local variable instead

Withdrawal memory withdrawal = $.withdrawals[withdrawalIdx];

// Check if withdrawal has already been completed (recipient is set to address(0) when completed)
require(withdrawal.recipient != address(0), WithdrawalAlreadyCompleted());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this necessary since we're already checking if the withdrawal is finalized?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disregard comment. This is useful to prevent cancelling the same withdrawal twice

@bxmmm1
Copy link
Collaborator Author

bxmmm1 commented Sep 25, 2025

The actual coverage of the contract is 96.21% (127/132) but for some reason the codecov is acting up

@bxmmm1 bxmmm1 marked this pull request as ready for review September 30, 2025 09:17
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.

2 participants