diff --git a/README.md b/README.md index e05f3b3..2f7657f 100644 --- a/README.md +++ b/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/ diff --git a/foundry.toml b/foundry.toml index 7670d09..7803d10 100644 --- a/foundry.toml +++ b/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}" diff --git a/package.json b/package.json index 7b10517..697ee7c 100644 --- a/package.json +++ b/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": { diff --git a/test/KSXVault.t.sol b/test/KSXVault.t.sol index a980305..bf8617a 100644 --- a/test/KSXVault.t.sol +++ b/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 {} } diff --git a/test/Upgrade.t.sol b/test/Upgrade.t.sol index 4add5f0..bb46281 100644 --- a/test/Upgrade.t.sol +++ b/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; diff --git a/test/utils/Bootstrap.sol b/test/utils/Bootstrap.sol index 7402cb7..2a65d79 100644 --- a/test/utils/Bootstrap.sol +++ b/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