🔎 add PR:REVIEW tags

README.md

@@ -4,6 +4,8 @@
 [![Foundry][foundry-badge]][foundry] 
 [![License: GPL-3.0][license-badge]][license]
 
+
+/** @PR:REVIEW update following urls **/
 [gha]: https://github.com/Kwenta/foundry-scaffold/actions
 [gha-badge]: https://github.com/Kwenta/foundry-scaffold/actions/workflows/test.yml/badge.svg
 [foundry]: https://getfoundry.sh/

foundry.toml

@@ -10,6 +10,13 @@ optimizer_runs = 1_000_000
 [fmt]
 line_length = 80
 number_underscore = "thousands"
+/** @PR:REVIEW recommend adding:
+multiline_func_header = "all"
+sort_imports = true
+contract_new_lines = true
+override_spacing = false
+wrap_comments = true
+**/
 
 [rpc_endpoints]
 mainnet = "${MAINNET_RPC_URL}" 

package.json

@@ -11,6 +11,7 @@
     "coverage:generate-lcov": "forge coverage --fork-url $(grep OPTIMISM_GOERLI_RPC_URL .env | cut -d '=' -f2) --report lcov",
     "analysis:slither": "slither .",
     "gas-snapshot": "forge snapshot --fork-url $(grep OPTIMISM_GOERLI_RPC_URL .env | cut -d '=' -f2)",
+    /** @PR:REVIEW synthetix not used **/
     "decode-custom-error": "npx @usecannon/cli decode synthetix-perps-market"
   },
   "repository": {

test/KSXVault.t.sol

@@ -5,6 +5,12 @@ import {Test} from "forge-std/Test.sol";
 import {KSXVault} from "../src/KSXVault.sol";
 import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
 
+/** @PR:REVIEW no tests for vault **/
+/** @PR:REVIEW add pDAO test **/
+/** @PR:REVIEW add share name **/
+/** @PR:REVIEW add share symbol **/
+/** @PR:REVIEW add use bootstrap everywhere instead of `Test` **/
+
 contract KSXVaultTest is Test {
     function setUp() public {}
 }

test/Upgrade.t.sol

@@ -7,10 +7,13 @@ import {MockVaultUpgrade} from "test/utils/mocks/MockVaultUpgrade.sol";
 
 contract UpgradeTest is Bootstrap {
     function setUp() public {
+        /** @PR:REVIEW unless there is a specific reason, avoid using fork tests **/
         initializeOptimism();
     }
 }
 
+/** @PR:REVIEW not entirely sure why you split up UpgradeTest/MockUpgrade/UpgradeVault; if no specific reason combine. otherwise thats fine **/
+
 contract MockUpgrade is UpgradeTest {
     MockVaultUpgrade mockVaultUpgrade;
 
@@ -22,6 +25,9 @@ contract MockUpgrade is UpgradeTest {
     }
 
     function test_upgrade() public {
+
+        /** @PR:REVIEW might as well fuzz the messages; never know what might be caught **/
+
         string memory message = "hi";
 
         bool success;

test/utils/Bootstrap.sol

@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-3.0-or-later
 pragma solidity 0.8.25;
 
+/** @PR:REVIEW remove any unused imports (except console bc it is helpful) **/
 import {console2} from "lib/forge-std/src/console2.sol";
 import {
     KSXVault