Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
139 changes: 139 additions & 0 deletions contracts/Vault4626.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
// SPDX-License-Identifier: BUSL-1.1
pragma solidity 0.8.24;

import {ERC4626} from "@openzeppelin/contracts/token/ERC20/extensions/ERC4626.sol";
import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
import {Math} from "@openzeppelin/contracts/utils/math/Math.sol";

/// @title Vault4626
/// @notice Minimal canonical ERC-4626 vault for the Sentrix ecosystem.
/// Accepts an underlying ERC-20 asset and mints vault shares (1:1 start).
/// @dev OpenZeppelin-derived. No yield strategy — yield sources extend this.
/// Inflation-attack guard: deploys with a small dead-share mint to make
/// donation attacks unprofitable (virtual offset pattern).
contract Vault4626 is ERC4626 {
using Math for uint256;

/// @notice Emitted when the vault receives a donation (pure asset transfer, no shares minted).
event Donation(address indexed donor, uint256 amount);

error Vault4626__ZeroDeposit();

/// @param _asset Underlying ERC-20 token address.
/// @param _name Vault share token name.
/// @param _symbol Vault share token symbol.
constructor(IERC20 _asset, string memory _name, string memory _symbol)
ERC4626(_asset)
ERC20(_name, _symbol)
{
// Inflation-attack guard: mint 1e3 dead shares to make donation attacks
// prohibitively expensive. With a 1e3 share floor, an attacker needs
// 1e3 * share_price of underlying to front-run, making the attack cost
// exceed any reasonable first-deposit value.
_mint(address(0xdead), 1e3);
}

// ─── Overrides for correct rounding ─────────────────────────────

/// @inheritdoc ERC4626
/// @dev Rounds UP (protects vault). Uses mulDiv with Math.Rounding.Ceil.
function _convertToShares(uint256 assets, Math.Rounding rounding)
internal
view
override
returns (uint256)
{
uint256 supply = totalSupply();
// If no live supply beyond our dead shares, shares = assets (1:1 after offset)
if (supply == 1e3) return assets;

return _tryMulDiv(assets, supply - 1e3, totalAssets(), rounding);
}
Comment on lines +42 to +53

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy lift

🧩 Analysis chain

🌐 Web query:

OpenZeppelin ERC4626 decimalsOffset virtual shares inflation attack protection pattern

💡 Result:

The ERC4626 inflation attack is a vulnerability where an attacker manipulates the vault's exchange rate by donating assets to an empty or near-empty vault, causing subsequent depositors to receive zero shares due to rounding [1][2]. OpenZeppelin mitigates this using a combination of virtual shares and a decimals offset [3][4]. The protection pattern works through two primary mechanisms: 1. Virtual Shares and Assets: The vault implementation adds a "virtual" quantity of shares and assets to the exchange rate computation [3]. By effectively seeding the vault with these virtual amounts, the vault enforces a non-zero exchange rate even when empty [3]. This ensures that any donation by an attacker is partially captured by the vault, making the attack significantly less profitable or entirely unprofitable [1][3]. 2. Decimals Offset: The _decimalsOffset function allows developers to increase the precision of share representation relative to the underlying asset [3]. By setting an offset (e.g., 9), the vault uses more decimal places for shares than the underlying token uses for assets [1][3]. This increased precision reduces rounding errors and forces an attacker to donate significantly larger amounts to achieve the same dilution effect, effectively making the attack prohibitively expensive [1][3]. Key considerations for this pattern include: - Default State: By default, OpenZeppelin's implementation may return 0 for _decimalsOffset, which provides minimal protection [5][6]. Developers are encouraged to override this function to return a higher value (e.g., 9) to enhance security [5][7][4]. - Trade-offs: While higher offsets increase security, they also change the decimal representation of the vault's shares, which may impact integration with other protocols [3][4]. - Inherent Loss: Because virtual shares and assets are included in the math, a small, usually negligible amount of value may be "captured" by the vault, making it unrecoverable by early depositors [3][4]. In summary, the combination of virtual shares and a non-zero decimals offset creates a robust defense by ensuring that the cost of an inflation attack is orders of magnitude higher than any potential gain [1][3].

Citations:


🏁 Script executed:

fd -t f "Vault4626.sol" --exec cat -n {} \;

Repository: sentrix-labs/canonical-contracts

Length of output: 6646


🏁 Script executed:

rg "function _tryMulDiv|function _convertToAssets" -A 5 contracts/

Repository: sentrix-labs/canonical-contracts

Length of output: 679


🏁 Script executed:

rg "function _convertToShares.*Math.Rounding" -A 8 contracts/ -t sol

Repository: sentrix-labs/canonical-contracts

Length of output: 104


🏁 Script executed:

grep -n "Rounds UP" contracts/Vault4626.sol

Repository: sentrix-labs/canonical-contracts

Length of output: 157


🏁 Script executed:

git log --oneline --all -S "supply - 1e3" 2>/dev/null | head -5 || echo "Git history check complete"

Repository: sentrix-labs/canonical-contracts

Length of output: 143


Inflation attack protection is ineffective — dead shares are excluded from the conversion formula.

The formula assets * (supply - 1e3) / totalAssets() subtracts dead shares from supply but does not add them to totalAssets. This asymmetry defeats the protection. Once any user deposits, subsequent depositors are vulnerable:

  1. Attacker deposits 1 wei → receives 1 share (1:1 since supply == 1e3)
  2. Attacker donates ~1e18 assets → supply = 1001, totalAssets = 1e18
  3. Victim deposits 1e18 → receives 1e18 * 1 / 1e18 = 1 share
  4. Victim holds 1/1002 of the vault; attacker redeems their share for nearly all assets

The OpenZeppelin virtual offset pattern mitigates this by adding the offset to both sides: assets * (supply + offset) / (totalAssets + offset). This ensures the offset continuously dilutes donation attacks throughout the vault's lifetime, not just initially.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@contracts/Vault4626.sol` around lines 42 - 53, The _convertToShares function
currently subtracts the dead-share offset from supply but does not add it to
totalAssets(), which breaks the virtual offset protection; update the conversion
to use the same offset on both sides (e.g., use (supply + OFFSET) and
(totalAssets() + OFFSET) where OFFSET is the existing dead-share constant 1e3)
so the return becomes _tryMulDiv(assets, supply + OFFSET, totalAssets() +
OFFSET, rounding) and retain the initial special-case check for supply == OFFSET
to preserve 1:1 behavior before live supply exists; ensure you reference the
existing supply variable, totalAssets() call, the OFFSET constant (1e3), and the
_tryMulDiv helper in the change.


/// @inheritdoc ERC4626
/// @dev Rounds DOWN (protects vault). Uses mulDiv with Math.Rounding.Floor.
function _convertToAssets(uint256 shares, Math.Rounding rounding)
internal
view
override
returns (uint256)
{
uint256 supply = totalSupply();
// If only dead shares exist, assets = shares (1:1)
if (supply == 1e3) return shares;

return _tryMulDiv(shares, totalAssets(), supply - 1e3, rounding);
}

/// @inheritdoc ERC4626
function totalAssets() public view override returns (uint256) {
return IERC20(asset()).balanceOf(address(this));
}

/// @inheritdoc ERC4626
function maxDeposit(address) public view override returns (uint256) {
return type(uint256).max;
}

/// @inheritdoc ERC4626
function maxMint(address) public view override returns (uint256) {
return type(uint256).max;
}

// ─── Hook pattern for yield extensions ──────────────────────────

/// @notice Hook called after shares are minted (deposit/mint).
/// @dev Override in yield-bearing children to route assets to strategies.
function _afterDeposit(address caller, uint256 assets, uint256 shares) internal virtual {}

/// @notice Hook called before shares are burned (withdraw/redeem).
/// @dev Override in yield-bearing children to pull assets from strategies.
function _beforeWithdraw(address caller, uint256 assets, uint256 shares) internal virtual {}
Comment on lines +91 to +93

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Document reentrancy considerations for _beforeWithdraw hook implementations.

The hook executes before _spendAllowance and _burn in _withdraw. Child contracts that make external calls here (e.g., pulling assets from strategies) create a reentrancy window. While the impact is limited since shares are eventually checked/burned, downstream implementers should be aware:

  /// `@notice` Hook called before shares are burned (withdraw/redeem).
  /// `@dev` Override in yield-bearing children to pull assets from strategies.
+ ///      WARNING: If making external calls in override, be aware this executes
+ ///      before shares are burned. Consider ReentrancyGuard for complex flows.
  function _beforeWithdraw(address caller, uint256 assets, uint256 shares) internal virtual {}

As per coding guidelines: "Reentrancy: state writes before external calls (CEI pattern); ReentrancyGuard where applicable."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@contracts/Vault4626.sol` around lines 91 - 93, Add documentation to the
_beforeWithdraw hook warning implementers that it executes before
_spendAllowance and _burn inside _withdraw, creating a potential reentrancy
window if implementations perform external calls (e.g., pulling from
strategies); instruct implementers to follow CEI (perform necessary state writes
in the parent or child before making external calls), avoid making external
calls that can re-enter vault logic, or protect implementations with
ReentrancyGuard where appropriate, and include a short example note describing
safe patterns for yield-bearing children.


// ─── Donation support ──────────────────────────────────────────

/// @notice Donate underlying assets to the vault. No shares minted in return.
/// Donations increase the share price for all existing shareholders.
function donate(uint256 assets) external {
if (assets == 0) revert Vault4626__ZeroDeposit();
SafeERC20.safeTransferFrom(IERC20(asset()), msg.sender, address(this), assets);
emit Donation(msg.sender, assets);
}

// ─── Internal ──────────────────────────────────────────────────

/// @dev Override deposit to call the after-hook.
function _deposit(address caller, address receiver, uint256 assets, uint256 shares) internal override {
SafeERC20.safeTransferFrom(IERC20(asset()), caller, address(this), assets);
_mint(receiver, shares);
_afterDeposit(caller, assets, shares);
emit Deposit(caller, receiver, assets, shares);
}

/// @dev Override withdraw to call the before-hook.
function _withdraw(address caller, address receiver, address owner, uint256 assets, uint256 shares)
internal
override
{
_beforeWithdraw(caller, assets, shares);
if (caller != owner) {
_spendAllowance(owner, caller, shares);
}
_burn(owner, shares);
SafeERC20.safeTransfer(IERC20(asset()), receiver, assets);
emit Withdraw(caller, receiver, owner, assets, shares);
}

/// @dev Safe mulDiv that returns 0 when operands overflow, with configurable rounding.
function _tryMulDiv(
uint256 x,
uint256 y,
uint256 denominator,
Math.Rounding rounding
) internal pure returns (uint256) {
if (denominator == 0) return 0;
return x.mulDiv(y, denominator, rounding);
}
}
Loading
Loading