Ghost-dao-contracts: vesting schedule check never binds (x1e9) and two guards were dropped - full allocation claimable on day one

Summary

GenesisClaim is the contract through which genesis members convert reserve into FTSO, rate-limited by two things: a schedule (a share of circulating supply) and a per-term maximum. Compared with the original this contract descends from (olympus-contracts, contracts/vesting/Genesis.sol), the shipped schedule math flips a division into a multiplication: circulating * percent / 1e6 became circulating * percent * 1e3, a factor of 1e9, and the clamp if (max > info.max) max = info.max was dropped. The claim guard compares that value against the requested mint, so the schedule check is vacuous: with reference-scale percent values a member can claim the entire per-term maximum on day one. Two smaller deviations ship with it, both reproduced: the wallet migration path lost its “receiver already has terms” guard (an existing term can be silently overwritten), and the claim-and-stake path lost its token approval (the function reverts on every call).

Everything below is inline: code, the full test, and the run output. Nothing to attach.

Finding 1 - the schedule check never binds (Medium, CVSS 6.5)

CVSS:3.1/AV:N/AC:L/PR:L/UI:N/S:U/C:N/I:H/A:N

Location: src/vesting/Genesis.sol, redeemableFor (lines 105-108) and the guard at the top of _claim.

Shipped:

uint max = circulatingSupply() * _terms[someAddress].percent * 1e3;
return (max - claimed(someAddress)) * 1e9;

Reference (olympus-contracts/contracts/vesting/Genesis.sol at main · OlympusDAO/olympus-contracts · GitHub, lines 160-165):

uint256 max = circulatingSupply().mul(info.percent).div(1e6);
if (max > info.max) max = info.max;
return max.sub(claimed(_address)).mul(1e9);

Factor difference: 1e9. The _claim guard divides the returned value by 1e9 and compares it to the requested mint, so with reference-scale percent values it passes for any mint up to the per-term maximum from the moment the term exists. The drip tied to circulating supply growth never constrains a claim. Dropping the clamp additionally makes the public view report an allowance above the schedule cap.

Reproduction excerpt (full test and output below):

[PASS] test_full_allocation_claimable_on_day_one()
  reference arithmetic binds: circulating * percent / 1e6 = 52,500e9 < cap 100,000e9
  shipped allowance on day one: >= cap
  end to end: 100,000e9 (the entire allocation) claimed in one call

Stated honestly: the surviving per-term maximum still caps each member, and the member pays reserve at treasury value, so this is premature release of vesting allocations and broken schedule integrity, not free minting. If the deployment rescaled percent values by 1e-9 the arithmetic would land near the reference; the dropped clamp and the changed guard semantics remain either way, and the fix below is the safe route.

Finding 2 - wallet migration can overwrite an existing term (Low)

pullWalletChange (lines 76-82). The reference requires the receiving address to have no terms (“Wallet already exists”); the shipped version only checks the push. A holder who already has a term can pull another and silently replace their own allocation: percent, claimed and max are all overwritten, while totalAllocated keeps counting both percents, so the allocation ledger drifts above reality. Reproduced in test 3.

Finding 3 - the claim-and-stake path reverts (Low)

stake() (lines 40-51). It hands the freshly minted FTSO to the staking contract, whose implementation pulls the token from the caller, but no approval for the staking contract exists anywhere in the contract (the old approval lines are commented out in this file). Every call reverts; claim() alone still works. Reproduced in test 2.

Reproduction - drop this file, run one command

Put the file below at test/vesting/GenesisGate.t.sol in the repository, then:

$ forge test --match-path "test/vesting/GenesisGate.t.sol" -vv

Expected output (also run against the shipped source as-is, no other changes):

[PASS] test_full_allocation_claimable_on_day_one() (gas: 187602)
[PASS] test_stake_path_reverts_without_approval() (gas: 1674432)
[PASS] test_wallet_change_overwrites_existing_terms() (gas: 112771)
Suite result: ok. 3 passed; 0 failed; 0 skipped

The test file:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;

import {Test} from "forge-std/Test.sol";
import {GenesisClaim} from "../../src/vesting/Genesis.sol";

// Minimal ERC20 with mint, enough for the claim flow.
contract Token {
    mapping(address => uint256) public balanceOf;
    mapping(address => mapping(address => uint256)) public allowance;

    function mint(address a, uint256 x) external {
        balanceOf[a] += x;
    }

    function approve(address s, uint256 x) external returns (bool) {
        allowance[msg.sender][s] = x;
        return true;
    }

    function transfer(address t, uint256 x) external returns (bool) {
        balanceOf[msg.sender] -= x;
        balanceOf[t] += x;
        return true;
    }

    function transferFrom(address f, address t, uint256 x) external returns (bool) {
        allowance[f][msg.sender] -= x;
        balanceOf[f] -= x;
        balanceOf[t] += x;
        return true;
    }

    function decimals() external pure returns (uint8) {
        return 9;
    }
}

// Treasury stub: deposit() mints FTSO to the caller (like the real one) and returns the value.
contract MockTreasury {
    Token public ftso;
    uint256 public circ;
    uint256 public fixedOut;

    constructor(Token _ftso, uint256 _circ, uint256 _fixedOut) {
        ftso = _ftso;
        circ = _circ;
        fixedOut = _fixedOut;
    }

    function baseSupply() external view returns (uint256) {
        return circ;
    }

    function deposit(address, uint256, uint256) external returns (uint256) {
        ftso.mint(msg.sender, fixedOut);
        return fixedOut;
    }
}

// gOHM-style wrapper stub with identity scaling (accounting only).
contract MockGhst {
    function balanceFrom(uint256 a) external pure returns (uint256) {
        return a;
    }

    function balanceTo(uint256 a) external pure returns (uint256) {
        return a;
    }
}

// Staking stub that pulls the FTSO from its caller, exactly like the real staking path.
contract MockStaking {
    Token public ftso;

    constructor(Token _ftso) {
        ftso = _ftso;
    }

    function stake(uint256 amount, address, bool, bool) external returns (uint256) {
        ftso.transferFrom(msg.sender, address(this), amount);
        return amount;
    }
}

contract GenesisGateTest is Test {
    Token ftso;
    Token reserve;
    MockTreasury treasury;
    MockGhst ghst;
    GenesisClaim genesis;

    address alice = address(0xA11CE);
    address dao = address(0xDA0);

    uint256 constant CIRC = 21_000_000e9;   // circulating supply, 9-dec units
    uint256 constant PERCENT = 2500;        // reference scale: per 1e6 (0.25%)
    uint256 constant MAXALLOC = 100_000e9;  // alice's absolute allocation cap

    function setUp() public {
        ftso = new Token();
        reserve = new Token();
        treasury = new MockTreasury(ftso, CIRC, MAXALLOC); // deposit returns the full cap
        ghst = new MockGhst();

        genesis = new GenesisClaim(
            address(ftso),
            address(reserve),
            address(treasury),
            dao,
            address(ghst),
            address(0xBEEF),
            1e12
        );
        genesis.setTerms(alice, PERCENT, 0, 0, MAXALLOC);
        reserve.mint(alice, 1_000_000e9);
    }

    function test_full_allocation_claimable_on_day_one() public {
        // What the schedule should allow first: circulating * percent / 1e6, clamped by max.
        uint256 referenceGate = CIRC * PERCENT / 1e6;
        if (referenceGate > MAXALLOC) referenceGate = MAXALLOC;
        assertLt(referenceGate, MAXALLOC); // 52,500e9 < 100,000e9: the schedule would bind

        // The shipped gate already exceeds the full cap on day one.
        uint256 gate = genesis.redeemableFor(alice) / 1e9;
        assertGe(gate, MAXALLOC);

        // The view also exceeds the cap (reference clamps it to info.max).
        assertGt(genesis.redeemableFor(alice) / 1e9, MAXALLOC);

        // End to end on the real contract: the entire allocation in one claim.
        vm.startPrank(alice);
        reserve.approve(address(genesis), MAXALLOC);
        genesis.claim(MAXALLOC);
        vm.stopPrank();

        assertEq(ftso.balanceOf(alice), MAXALLOC); // full allocation received at once
        assertEq(genesis.claimed(alice), MAXALLOC); // accounting recorded
    }

    function test_stake_path_reverts_without_approval() public {
        MockStaking staking = new MockStaking(ftso);
        GenesisClaim g2 = new GenesisClaim(
            address(ftso),
            address(reserve),
            address(treasury),
            dao,
            address(ghst),
            address(staking),
            1e12
        );
        g2.setTerms(alice, PERCENT, 0, 0, MAXALLOC);

        vm.startPrank(alice);
        reserve.approve(address(g2), MAXALLOC);
        vm.expectRevert();
        g2.stake(alice, MAXALLOC, false, false); // staking pulls the FTSO, no approval exists
        vm.stopPrank();
    }

    function test_wallet_change_overwrites_existing_terms() public {
        address carol = address(0xCA501);
        address dave = address(0xDABE);

        genesis.setTerms(carol, 100, 0, 0, 5_000e9);
        genesis.setTerms(dave, 200, 0, 0, 9_000e9);

        vm.prank(carol);
        genesis.pushWalletChange(dave);

        vm.prank(dave);
        genesis.pullWalletChange(carol); // reference guards this with "Wallet already exists"

        // dave's own allocation was silently replaced by carol's term.
        GenesisClaim.Term memory t = genesis.terms(dave);
        assertEq(t.percent, 100);
        assertEq(t.max, 5_000e9);
    }
}

Impact

  • Finding 1: premature release of vesting allocations. With reference-scale percent configuration the schedule gate never constrains a claim, so any member with terms can withdraw their entire allocation in a single claim instead of waiting for circulating supply growth. Bounded by each term’s max and by totalAllocated; the member pays reserve at treasury value.
  • Finding 2: a member’s own allocation can be silently replaced during a wallet change while the allocation ledger keeps counting both percents.
  • Finding 3: the claim-and-stake convenience path is unusable; every call reverts.

Suggested fix

Restore the reference schedule math and the clamp:

uint256 max = circulatingSupply() * _terms[someAddress].percent / 1e6;
if (max > _terms[someAddress].max) max = _terms[someAddress].max;

Then restore the pullWalletChange guard (revert when the receiver already has terms) and approve the staking contract inside stake() (or reinstate the one-time approval function). A regression test with a small circulating supply and a large max pins all three.

What I checked and could not break

Checkpoint library strides and lookups; the BIP-340 verifier bounds and message binding (the unregistered-key path cannot pass); treasury refund and buyback conversions; the shares token math; the warmup rounding; gatekeeper key handling and chain binding. The slow-clap pallet is not wired into any runtime; the AfterParty contracts referenced by the deploy runner do not exist in the repository.

Disclosure

This stays with the GHOST team until the fix is in or you tell me otherwise. It has not been published anywhere else.

Many thanks for your work!

It looks like it was not stated anywhere that src/vesting/Governance.sol is not used in the current version of the project and is not planned to be used anymore. We now use genesis preclaims on the ghost chain side itself, so this file exists here only as Olympus DAO V2 legacy.

I personally do not think this is a bug, since this code will never be used. In any case, you found a valid pain point about marking which code is deprecated and which is used especially in smart contracts