Skip to content

Address audit findings #10

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
cxkoda merged 3 commits into from
Mar 4, 2026
Merged

Address audit findings #10

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0c204b0
Address audit findings
cxkoda Mar 3, 2026
1baf31c
fix refunds check
cxkoda Mar 4, 2026
304f75b
optimize for size
cxkoda Mar 4, 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
2 changes: 1 addition & 1 deletion foundry.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ out = "out"
libs = ["lib"]

optimizer = true
optimizer_runs = 200
optimizer_runs = 1

# See more config options https://github.com/foundry-rs/foundry/blob/master/crates/config/README.md#all-options

Expand Down
65 changes: 54 additions & 11 deletions src/sales/SettlementSale.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ import {TokenAmount, WalletTokenAmount} from "sales/interfaces/types.sol";
/// The `entityID` refers to an entity in the Sonar system, which can be either a legal entity or an individual.
/// A wallet is an address used to commit funds to the sale.
/// An entity can have multiple wallets, but each wallet is associated with exactly one entity.
/// All wallets under the same entity are mutually trusted: any wallet can cancel or reduce commitments
/// for any other wallet in the entity. Funds are always returned to the committing wallet, not the caller.
///
/// With the exception of the emergency recovery mechanism, tokens can only be:
/// - transferred to the contract as part of a bid
Expand Down Expand Up @@ -152,6 +154,10 @@ contract SettlementSale is
/// @notice The role allowed to refund entities.
bytes32 public constant REFUNDER_ROLE = keccak256("REFUNDER_ROLE");

/// @notice The role allowed to reduce commitments on behalf of entities.
/// @dev This is not granted by default. It should be granted manually by the DEFAULT_ADMIN_ROLE when needed.
bytes32 public constant COMMITMENT_REDUCER_ROLE = keccak256("COMMITMENT_REDUCER_ROLE");

// Initialization errors
error InvalidPaymentTokenDecimals(address token, uint256 got, uint256 want);
error DuplicatePaymentToken(address token);
Expand Down Expand Up @@ -188,8 +194,8 @@ contract SettlementSale is

// Cancellation errors
error ReduceCommitmentDisabled();
error ReductionExceedsCommitment(
bytes16 entityID, address wallet, address token, uint256 amount, uint256 committed
error ReductionExceedsReducibleAmount(
bytes16 entityID, address wallet, address token, uint256 amount, uint256 committed, uint256 allocated
);

// Refund errors
Expand Down Expand Up @@ -508,10 +514,16 @@ contract SettlementSale is
}

for (uint256 i = 0; i < init.extraManagers.length; i++) {
if (init.extraManagers[i] == address(0)) {
revert ZeroAddress();
}
_grantRole(SALE_MANAGER_ROLE, init.extraManagers[i]);
}

for (uint256 i = 0; i < init.extraPausers.length; i++) {
if (init.extraPausers[i] == address(0)) {
revert ZeroAddress();
}
_grantRole(PAUSER_ROLE, init.extraPausers[i]);
}

Expand Down Expand Up @@ -645,6 +657,7 @@ contract SettlementSale is

/// @notice Processes a bid during the `Commitment` stage, validating the purchase permit, any constraints specified on the permit, and updating the bid.
/// @dev The minimum and maximum total bid amount and the minimum and maximum price are specified on the purchase permit (`minAmount`, `maxAmount`, `minPrice`, and `maxPrice`, respectively).
/// `minAmount` is enforced only at bid submission. It is not stored onchain and does not constrain subsequent reductions during the `Cancellation` stage.
function _processBid(
IERC20 token,
Bid calldata newBid,
Expand Down Expand Up @@ -687,8 +700,9 @@ contract SettlementSale is
}

EntityState storage state = _entityStateByID[purchasePermit.saleSpecificEntityID];
// additional safety check: to avoid any bookkeeping issues, we disallow new bids for entities that have already been refunded.
// this can theoretically happen if the commitment stage was reopened after already refunding some entities.
// since already refunded entities cannot be refunded again, we disallow new bids for them to avoid any bookkeeping issues.
// while this cannot happen in the normal flow of the sale, it is theoretically possible if the commitment stage is reopened
// through unsafeSetStage after some entities have already been refunded.
if (state.refunded) {
revert AlreadyRefunded(purchasePermit.saleSpecificEntityID);
}
Expand Down Expand Up @@ -754,8 +768,10 @@ contract SettlementSale is
_setStage(Stage.Cancellation);
}

/// @notice Fully cancels an entity's bid during the `Cancellation` stage, refunding all committed amounts.
/// @dev Can be called by any wallet associated with the entity. Always available regardless of `reduceCommitmentEnabled`.
/// @notice Fully cancels an entity's bid during the `Cancellation` stage, refunding all unaccepted committed amounts.
/// @dev Can be called by any wallet associated with the entity.
/// Always available regardless of `reduceCommitmentEnabled`.
/// For intentional partial reductions, use `reduceCommitment()` instead.
function cancelBid() external onlyStage(Stage.Cancellation) onlyUnpaused {
bytes16 entityID = _entityIDByAddress[msg.sender];
if (entityID == bytes16(0)) {
Expand All @@ -772,9 +788,11 @@ contract SettlementSale is
uint256 numTokens = _paymentTokens.length;
for (uint256 i = 0; i < wallets.length; i++) {
WalletState storage walletState = state.walletStates[wallets[i]];

for (uint256 j = 0; j < numTokens; j++) {
IERC20 token = _paymentTokens[j];
uint256 amount = walletState.committedAmountByToken[token];
uint256 amount = walletState.committedAmountByToken[token] - walletState.acceptedAmountByToken[token];

if (amount > 0) {
_reduceCommitment(entityID, wallets[i], token, amount);
}
Expand All @@ -783,7 +801,12 @@ contract SettlementSale is
}

/// @notice Partially reduces specific wallet/token commitments during the `Cancellation` stage.
/// @dev Only processes the caller-supplied tuples. Any wallet/token pairs not included in `reductions` are left
/// unchanged, and the entity's remaining committed balance will be carried into settlement.
/// For full cancellation, use `cancelBid()` which reads all pairs from contract storage.
/// @dev Requires `reduceCommitmentEnabled`. The caller must be a wallet associated with the entity.
/// @dev No minimum floor is enforced on the resulting commitment. The `minAmount` constraint from the purchase permit
/// applies only at bid submission and entities may reduce below that threshold here.
/// @param reductions Array of (wallet, token, amount) tuples specifying what to reduce.
function reduceCommitment(WalletTokenAmount[] calldata reductions)
external
Expand Down Expand Up @@ -813,6 +836,9 @@ contract SettlementSale is
/// @notice Reduces a wallet's commitment for a given token by `amount` and transfers the funds back.
function _reduceCommitment(bytes16 entityID, address wallet, IERC20 token, uint256 amount) internal {
EntityState storage state = _entityStateByID[entityID];
if (state.refunded) {
revert AlreadyRefunded(entityID);
}

if (!state.wallets.contains(wallet)) {
revert WalletNotAssociatedWithEntity(wallet, entityID);
Expand All @@ -827,10 +853,10 @@ contract SettlementSale is
}

WalletState storage walletState = state.walletStates[wallet];
if (walletState.committedAmountByToken[token] < amount) {
revert ReductionExceedsCommitment(
entityID, wallet, address(token), amount, walletState.committedAmountByToken[token]
);
uint256 committed = walletState.committedAmountByToken[token];
uint256 allocated = walletState.acceptedAmountByToken[token];
if (amount > committed - allocated) {
revert ReductionExceedsReducibleAmount(entityID, wallet, address(token), amount, committed, allocated);
}

walletState.committedAmountByToken[token] -= amount;
Expand Down Expand Up @@ -1328,11 +1354,28 @@ contract SettlementSale is

/// @notice Recovers any ERC20 tokens that are sent to the contract.
/// @dev This can be used to recover any tokens that are sent to the contract by mistake.
/// Use `forceReduceCommitment` which updates accounting state instead if possible.
function recoverTokens(IERC20 token, uint256 amount, address to) external onlyRole(TOKEN_RECOVERER_ROLE) {
emit TokensRecovered(address(token), amount, to);
token.safeTransfer(to, amount);
}

/// @notice Sale operator initiated reduction of wallet commitments, bypassing stage and pause restrictions.
/// @dev This should be preferred over `recoverTokens()` for committed payment tokens, since it
/// correctly updates all accounting state (committed/cancelled amounts, bid totals, global counters).
function forceReduceCommitment(WalletTokenAmount[] calldata reductions) external onlyRole(COMMITMENT_REDUCER_ROLE) {
for (uint256 i = 0; i < reductions.length; i++) {
WalletTokenAmount calldata c = reductions[i];

bytes16 entityID = _entityIDByAddress[c.wallet];
if (entityID == bytes16(0)) {
revert WalletNotInitialized(c.wallet);
}

_reduceCommitment(entityID, c.wallet, IERC20(c.token), c.amount);
}
}

/// @notice Checks if the contract supports an interface.
function supportsInterface(bytes4 interfaceId)
public
Expand Down
Loading