Harden OKT audit findings and expand test coverage

foundry.toml

@@ -2,6 +2,7 @@
 src = "src"
 out = "out"
 libs = ["lib"]
+evm_version = "shanghai"
 
 # See more config options https://github.com/foundry-rs/foundry/blob/master/crates/config/README.md#all-options
 remappings = ['@openzeppelin/contracts/=lib/openzeppelin-contracts/contracts/']

src/OriginKeyToken.sol

@@ -88,8 +88,8 @@ pragma solidity 0.8.20;
  *                  - sum(payoutsOf)
  *
  *  Every function maintains this equation. The sell function is the most
- *  critical — it must use signedSub with taxed included in payout, and
- *  must distribute the fee AFTER updating payoutsOf. This is PITcoin exact.
+ *  critical — it must release only the dividend baseline for burned tokens,
+ *  distribute the fee after updating payoutsOf, and pay net cbBTC directly.
  *
  *  payoutsOf is int256 — it CAN and WILL go negative. This is correct.
  *  DO NOT change to uint256. DO NOT change signedSub to signedAdd in sell.
@@ -108,6 +108,7 @@ pragma solidity 0.8.20;
  */
 
 import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
+import "@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol";
 import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
 import "@openzeppelin/contracts/utils/ReentrancyGuard.sol";
 
@@ -131,6 +132,7 @@ contract OriginKeyToken is ReentrancyGuard {
     uint256 public constant SELL_FEE = 7;
     uint256 public constant INSCRIBE_FEE = 7;
     uint256 public constant MIN_SATS = 100;
+    uint256 public constant MIN_SELL = 100;
     uint256 public constant MAX_BUY = 1_000_000; // 0.01 BTC per transaction
 
     // ─── Dividend accumulator — PITcoin exact ────────────────────────────────
@@ -176,6 +178,7 @@ contract OriginKeyToken is ReentrancyGuard {
     // ─── Constructor ──────────────────────────────────────────────────────────
     constructor(address _cbbtc) {
         require(_cbbtc != address(0), "cbBTC: zero address");
+        require(IERC20Metadata(_cbbtc).decimals() == 8, "cbBTC: expected 8 decimals");
         CBBTC = IERC20(_cbbtc);
         vaultRegistrar = msg.sender;
         ordinalOracle = msg.sender;
@@ -251,16 +254,17 @@ contract OriginKeyToken is ReentrancyGuard {
     // When selling `tokens` for `taxed` cbBTC:
     //   - totalSupply decreases by tokens
     //   - cbbtcInContract decreases by taxed
-    //   - To maintain the equation, payoutsOf must decrease by:
-    //     taxed + (tokens * profitPerToken) / MAGNITUDE
-    //   - signedSub DECREASES payoutsOf (makes it more negative)
+    //   - Direct cbBTC payment handles `taxed`
+    //   - payoutsOf must decrease only by the burned-token dividend baseline:
+    //     (tokens * profitPerToken) / MAGNITUDE
+    //   - Including `taxed` in payoutsOf would double-count seller proceeds
     //   - _distributeFee happens AFTER payoutsOf update
     //
     // DO NOT change signedSub to signedAdd — that breaks dividend distribution
     // for any wallet that sells and rebuys. PITcoin uses signedSub. Always.
     //
     function sell(uint256 tokens, uint256 minCbbtc) external nonReentrant {
-        require(tokens > 0, "Zero tokens");
+        require(tokens >= MIN_SELL, "Minimum 100 sats to sell");
         require(balanceOf[msg.sender] >= tokens, "Insufficient balance");
         require(totalSupply > tokens, "Cannot sell entire supply");
 
@@ -293,6 +297,7 @@ contract OriginKeyToken is ReentrancyGuard {
     // ─── Transfer — zero fee ──────────────────────────────────────────────────
     function transfer(address to, uint256 tokens) external returns (bool) {
         require(to != address(0), "Zero address");
+        require(tokens > 0, "Zero tokens");
         require(balanceOf[msg.sender] >= tokens, "Insufficient balance");
 
         _checkVaultSweep(msg.sender, tokens);
@@ -377,6 +382,8 @@ contract OriginKeyToken is ReentrancyGuard {
         vaultHasOrdinal[vault] = (ordinalNumber > 0);
 
         if (ordinalNumber > 0) {
+            require(ordinalVaultAddress[ordinalNumber] == address(0), "Ordinal: already registered");
+            require(!ordinalHasBeenMoved[ordinalNumber], "Ordinal: already moved");
             vaultOrdinal[vault] = ordinalNumber;
             ordinalVaultAddress[ordinalNumber] = vault;
         }

test/OKT.audit.t.sol

@@ -17,6 +17,14 @@ contract MockCbBTC is ERC20 {
     }
 }
 
+contract MockBadDecimals is ERC20 {
+    constructor() ERC20("Bad BTC", "BADBTC") {}
+
+    function decimals() public pure override returns (uint8) {
+        return 18;
+    }
+}
+
 // Malicious contract that tries reentrancy
 contract ReentrancyAttacker {
     OriginKeyToken public okt;
@@ -87,8 +95,6 @@ contract OKTAuditTest is Test {
         vm.prank(bob);
         okt.buy(100_000, 0);
 
-        // Attacker tries to reenter — ReentrancyGuard should block
-        uint256 contractBefore = cbbtc.balanceOf(address(okt));
         // The reentrancy attempt happens inside withdraw via fallback
         // ReentrancyGuard prevents double withdrawal
         uint256 contractAfter = cbbtc.balanceOf(address(okt));
@@ -314,23 +320,24 @@ contract OKTAuditTest is Test {
     // IMMUTABILITY — verify no admin functions exist
     // ═══════════════════════════════════════════════════════════════════════════
 
-    function test_registrarCannotChangeAfterDeploy() public {
+    function test_registrarCannotChangeAfterDeploy() public view {
         address reg = okt.vaultRegistrar();
         assertEq(reg, address(this), "Registrar should be deployer");
         // No function exists to change registrar — this is verified by the
         // contract not having a setRegistrar() function
     }
 
-    function test_oracleCannotChangeAfterDeploy() public {
+    function test_oracleCannotChangeAfterDeploy() public view {
         address oracle = okt.ordinalOracle();
         assertEq(oracle, address(this), "Oracle should be deployer");
         // No function exists to change oracle — immutable by design
     }
 
-    function test_feeCannotBeChanged() public {
+    function test_feeCannotBeChanged() public view {
         assertEq(okt.BUY_FEE(), 7);
         assertEq(okt.SELL_FEE(), 7);
         assertEq(okt.INSCRIBE_FEE(), 7);
+        assertEq(okt.MIN_SELL(), 100);
         // Constants — cannot be changed after deploy
     }
 
@@ -369,20 +376,47 @@ contract OKTAuditTest is Test {
         okt.inscribe(vault1, bytes32("TEST"), 99, 0);
     }
 
-    function test_sellOneToken() public {
+    function test_sellBelowMinimumReverts() public {
+        vm.prank(alice);
+        okt.buy(10_000, 0);
+        vm.prank(bob);
+        okt.buy(10_000, 0);
+
+        vm.prank(bob);
+        vm.expectRevert("Minimum 100 sats to sell");
+        okt.sell(99, 0);
+    }
+
+    function test_sellMinimumChargesFee() public {
         vm.prank(alice);
         okt.buy(10_000, 0);
         vm.prank(bob);
         okt.buy(10_000, 0);
 
         uint256 bobBefore = cbbtc.balanceOf(bob);
         vm.prank(bob);
-        okt.sell(1, 0);
+        okt.sell(100, 0);
         uint256 bobAfter = cbbtc.balanceOf(bob);
 
-        // Selling 1 token: fee = 0 (rounds down), so seller gets 1 sat
-        // But 7% of 1 = 0.07 which rounds to 0, so taxed = 1
-        assertGe(bobAfter, bobBefore, "Selling 1 token should return at least 0 sats");
+        assertEq(bobAfter - bobBefore, 93, "Minimum sell should charge 7 sats");
+    }
+
+    function test_constructorRejectsNonEightDecimalReserve() public {
+        MockBadDecimals badToken = new MockBadDecimals();
+
+        vm.expectRevert("cbBTC: expected 8 decimals");
+        new OriginKeyToken(address(badToken));
+    }
+
+    function test_erc20AllowanceFunctionsAreNotImplemented() public {
+        (bool approveOk,) = address(okt).call(abi.encodeWithSignature("approve(address,uint256)", alice, 1));
+        (bool allowanceOk,) = address(okt).staticcall(abi.encodeWithSignature("allowance(address,address)", alice, bob));
+        (bool transferFromOk,) =
+            address(okt).call(abi.encodeWithSignature("transferFrom(address,address,uint256)", alice, bob, 1));
+
+        assertFalse(approveOk, "approve should not be exposed");
+        assertFalse(allowanceOk, "allowance should not be exposed");
+        assertFalse(transferFromOk, "transferFrom should not be exposed");
     }
 
     function test_multipleVaultsEarnProportionally() public {

test/OKT.invariant.t.sol

@@ -26,10 +26,12 @@ contract OKTHandler is Test {
     MockCbBTC public cbbtc;
 
     address[] public actors;
+    address[] public vaults;
     mapping(address => bool) public isActor;
 
     uint256 constant MIN_SATS = 100;
     uint256 constant MAX_SATS = 1_000_000; // 0.01 BTC max per action
+    uint256 constant MAX_ORDINAL = 1_000_000;
 
     constructor(OriginKeyToken _okt, MockCbBTC _cbbtc) {
         okt = _okt;
@@ -44,6 +46,15 @@ contract OKTHandler is Test {
             vm.prank(actor);
             cbbtc.approve(address(okt), type(uint256).max);
         }
+
+        for (uint256 i = 0; i < 3; i++) {
+            vaults.push(makeAddr(string(abi.encodePacked("vault", i))));
+        }
+
+        address registrar = okt.vaultRegistrar();
+        cbbtc.mint(registrar, 30_000_000);
+        vm.prank(registrar);
+        cbbtc.approve(address(okt), type(uint256).max);
     }
 
     // ─── Buy ──────────────────────────────────────────────────────────────────
@@ -61,9 +72,9 @@ contract OKTHandler is Test {
     function sell(uint256 actorSeed, uint256 amount) external {
         address actor = actors[actorSeed % actors.length];
         uint256 balance = okt.balanceOf(actor);
-        if (balance < 2) return; // need at least 2 to sell 1 and keep supply > tokens
+        if (balance < okt.MIN_SELL()) return;
 
-        amount = bound(amount, 1, balance - 1);
+        amount = bound(amount, okt.MIN_SELL(), balance);
 
         // Make sure totalSupply > amount
         if (okt.totalSupply() <= amount) return;
@@ -72,6 +83,19 @@ contract OKTHandler is Test {
         try okt.sell(amount, 0) {} catch {}
     }
 
+    // ─── Vault Sell ──────────────────────────────────────────────────────────
+    function vaultSell(uint256 vaultSeed, uint256 amount) external {
+        address vault = vaults[vaultSeed % vaults.length];
+        uint256 balance = okt.balanceOf(vault);
+        if (balance < okt.MIN_SELL()) return;
+
+        amount = bound(amount, okt.MIN_SELL(), balance);
+        if (okt.totalSupply() <= amount) return;
+
+        vm.prank(vault);
+        try okt.sell(amount, 0) {} catch {}
+    }
+
     // ─── Withdraw ─────────────────────────────────────────────────────────────
     function withdraw(uint256 actorSeed) external {
         address actor = actors[actorSeed % actors.length];
@@ -81,6 +105,15 @@ contract OKTHandler is Test {
         try okt.withdraw() {} catch {}
     }
 
+    // ─── Vault Withdraw ──────────────────────────────────────────────────────
+    function vaultWithdraw(uint256 vaultSeed) external {
+        address vault = vaults[vaultSeed % vaults.length];
+        if (okt.dividendsOf(vault) == 0) return;
+
+        vm.prank(vault);
+        try okt.withdraw() {} catch {}
+    }
+
     // ─── Reinvest ─────────────────────────────────────────────────────────────
     function reinvest(uint256 actorSeed) external {
         address actor = actors[actorSeed % actors.length];
@@ -104,21 +137,72 @@ contract OKTHandler is Test {
         try okt.transfer(to, amount) {} catch {}
     }
 
+    // ─── Vault Transfer ──────────────────────────────────────────────────────
+    function vaultTransfer(uint256 vaultSeed, uint256 actorSeed, uint256 amount) external {
+        address from = vaults[vaultSeed % vaults.length];
+        address to = actors[actorSeed % actors.length];
+
+        uint256 balance = okt.balanceOf(from);
+        if (balance == 0) return;
+        amount = bound(amount, 1, balance);
+
+        vm.prank(from);
+        try okt.transfer(to, amount) {} catch {}
+    }
+
+    // ─── Inscribe ────────────────────────────────────────────────────────────
+    function inscribe(uint256 vaultSeed, uint256 amount, uint256 ordinalSeed) external {
+        address vault = vaults[vaultSeed % vaults.length];
+        if (okt.isVault(vault)) return;
+
+        amount = bound(amount, MIN_SATS, MAX_SATS);
+        uint256 ordinalNumber = bound(ordinalSeed, 0, MAX_ORDINAL);
+        if (ordinalNumber > 0) {
+            if (okt.ordinalVaultAddress(ordinalNumber) != address(0)) return;
+            if (okt.ordinalHasBeenMoved(ordinalNumber)) return;
+        }
+
+        address registrar = okt.vaultRegistrar();
+        if (cbbtc.balanceOf(registrar) < amount) return;
+
+        vm.prank(registrar);
+        try okt.inscribe(vault, bytes32(uint256(uint160(vault))), amount, ordinalNumber) {} catch {}
+    }
+
+    // ─── Report Ordinal Moved ────────────────────────────────────────────────
+    function reportOrdinalMoved(uint256 ordinalSeed) external {
+        uint256 ordinalNumber = bound(ordinalSeed, 1, MAX_ORDINAL);
+        if (okt.ordinalHasBeenMoved(ordinalNumber)) return;
+
+        vm.prank(okt.ordinalOracle());
+        try okt.reportOrdinalMoved(ordinalNumber) {} catch {}
+    }
+
     // ─── Helper for invariant checks ──────────────────────────────────────────
     function allActors() external view returns (address[] memory) {
         return actors;
     }
 
+    function allVaults() external view returns (address[] memory) {
+        return vaults;
+    }
+
     function totalClaimable() external view returns (uint256 total) {
         for (uint256 i = 0; i < actors.length; i++) {
             total += okt.dividendsOf(actors[i]);
         }
+        for (uint256 i = 0; i < vaults.length; i++) {
+            total += okt.dividendsOf(vaults[i]);
+        }
     }
 
     function totalOKTBalance() external view returns (uint256 total) {
         for (uint256 i = 0; i < actors.length; i++) {
             total += okt.balanceOf(actors[i]);
         }
+        for (uint256 i = 0; i < vaults.length; i++) {
+            total += okt.balanceOf(vaults[i]);
+        }
     }
 }
 
@@ -163,11 +247,16 @@ contract OKTInvariantTest is Test {
     // No actor should be able to claim more than the contract holds
     function invariant_noPhantomDividends() public view {
         address[] memory actors = handler.allActors();
+        address[] memory vaults = handler.allVaults();
         uint256 contractBalance = cbbtc.balanceOf(address(okt));
         for (uint256 i = 0; i < actors.length; i++) {
             uint256 divs = okt.dividendsOf(actors[i]);
             assertLe(divs, contractBalance, "Single actor dividends exceed contract balance");
         }
+        for (uint256 i = 0; i < vaults.length; i++) {
+            uint256 divs = okt.dividendsOf(vaults[i]);
+            assertLe(divs, contractBalance, "Single vault dividends exceed contract balance");
+        }
     }
 
     // ─── INVARIANT 4: PROFIT PER TOKEN NEVER DECREASES ───────────────────────