Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upd forge-std and fix all compiler warnings #388

Merged
merged 3 commits into from
Jan 27, 2025
Merged
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
2 changes: 1 addition & 1 deletion .foundryref
Original file line number Diff line number Diff line change
@@ -1 +1 @@
03ea54c
v0.3.0
13 changes: 7 additions & 6 deletions .github/workflows/regular-mainnet.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ jobs:
cache-path: ${{ steps.cache.outputs.path }}
cache-key: ${{ steps.cache.outputs.key }}
env:
FORGE_REV: 03ea54c
JUST_TAG: 1.24.0
steps:
- name: Build cache params
Expand All @@ -35,7 +34,6 @@ jobs:
CACHE_PATH: |
~/.cargo/bin/
KEY_INPUT: |
forge:${{env.FORGE_REV}}
just:${{env.JUST_TAG}}

- uses: actions/cache@v4
Expand All @@ -48,14 +46,12 @@ jobs:
run: cargo install "just@$JUST_TAG"
if: steps.get-cache.outputs.cache-hit != 'true'

- name: Install forge & anvil
run: cargo install --git https://github.com/foundry-rs/foundry --rev "$FORGE_REV" --profile release --locked forge anvil
if: steps.get-cache.outputs.cache-hit != 'true'

test:
name: Integration & Invariant tests
runs-on: ubuntu-latest
needs: bootstrap
env:
FORGE_REV: v0.3.0
steps:
- uses: actions/checkout@v4
with:
Expand All @@ -66,6 +62,11 @@ jobs:
path: ${{ needs.bootstrap.outputs.cache-path }}
key: ${{ needs.bootstrap.outputs.cache-key }}

- name: Install Foundry
uses: foundry-rs/foundry-toolchain@v1
with:
version: ${{ env.FORGE_REV }}

- name: Install node
uses: actions/setup-node@v4
with:
Expand Down
13 changes: 7 additions & 6 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ jobs:
cache-path: ${{ steps.cache.outputs.path }}
cache-key: ${{ steps.cache.outputs.key }}
env:
FORGE_REV: 03ea54c
JUST_TAG: 1.24.0
steps:
- name: Build cache params
Expand All @@ -42,7 +41,6 @@ jobs:
CACHE_PATH: |
~/.cargo/bin/
KEY_INPUT: |
forge:${{env.FORGE_REV}}
just:${{env.JUST_TAG}}

- uses: actions/cache@v4
Expand All @@ -55,10 +53,6 @@ jobs:
run: cargo install "just@$JUST_TAG"
if: steps.get-cache.outputs.cache-hit != 'true'

- name: Install forge & anvil
run: cargo install --git https://github.com/foundry-rs/foundry --rev "$FORGE_REV" --profile release --locked forge anvil
if: steps.get-cache.outputs.cache-hit != 'true'

linters:
name: Linters
runs-on: ubuntu-latest
Expand Down Expand Up @@ -90,6 +84,8 @@ jobs:
name: Foundry project
runs-on: ubuntu-latest
needs: bootstrap
env:
FORGE_REV: v0.3.0
steps:
- uses: actions/checkout@v4
with:
Expand All @@ -100,6 +96,11 @@ jobs:
path: ${{ needs.bootstrap.outputs.cache-path }}
key: ${{ needs.bootstrap.outputs.cache-key }}

- name: Install Foundry
uses: foundry-rs/foundry-toolchain@v1
with:
version: ${{ env.FORGE_REV }}

- name: Install node
uses: actions/setup-node@v4
with:
Expand Down
7 changes: 4 additions & 3 deletions Justfile
Original file line number Diff line number Diff line change
Expand Up @@ -156,9 +156,10 @@ _deploy-prod *args:
forge script {{deploy_script_path}} --sig="run(string)" --force --rpc-url ${RPC_URL} {{args}} -- `git rev-parse HEAD`

_deploy-impl *args:
forge script {{deploy_impls_script_path}} --sig="deploy(string,string)" \
--rpc-url {{anvil_rpc_url}} --slow {{args}} \
-- {{deploy_config_path}} `git rev-parse HEAD`
FOUNDRY_PROFILE=deploy-impl \
forge script {{deploy_impls_script_path}} --sig="deploy(string,string)" \
--rpc-url {{anvil_rpc_url}} --slow {{args}} \
-- {{deploy_config_path}} `git rev-parse HEAD`

[confirm("You are about to broadcast deployment transactions to the network. Are you sure?")]
deploy-impl-prod *args:
Expand Down
7 changes: 6 additions & 1 deletion foundry.toml
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ fs_permissions = [
{ access = "read", path = "./test/fixtures" },
]

ignored_warnings_from = ["src/lib/base-oracle/HashConsensus.sol", "test/helpers/mocks/LidoMock.sol", "test/OssifiableProxy.t.sol"]
ignored_warnings_from = ["src/lib/base-oracle/HashConsensus.sol", "test/OssifiableProxy.t.sol"]

[profile.ci]
verbosity = 3
Expand All @@ -47,3 +47,8 @@ fuzz = { runs = 10_000, max_test_rejects = 2_000_000 }
# due to coverage running with optimizer disabled, we need to increase the gas limit
# to fit CSM contract tests in one block
block_gas_limit = 60_000_000

[profile.deploy-impl]
# unknown problem of too high estimated gas usage in the implementation deployment script
# after upgrading forge-std 1.7.6 -> 1.9.5
block_gas_limit = 60_000_000
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
"@openzeppelin/contracts-upgradeable": "5.0.2",
"@openzeppelin/merkle-tree": "^1.0.6",
"ds-test": "https://github.com/dapphub/ds-test",
"forge-std": "https://github.com/foundry-rs/forge-std.git#v1.7.6"
"forge-std": "https://github.com/foundry-rs/forge-std.git#v1.9.5"
},
"devDependencies": {
"husky": "^8.0.3",
Expand Down
4 changes: 2 additions & 2 deletions script/fork-helpers/NodeOperators.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -297,15 +297,15 @@ contract NodeOperators is

bytes3 moduleId = bytes3(uint24(_getCSMId()));
bytes5 nodeOpId = bytes5(uint40(noId));
bytes8 validatorIndex = bytes8(uint64(validatorIndex));
bytes8 _validatorIndex = bytes8(uint64(validatorIndex));

(, uint256 refSlot, , ) = vebo.getConsensusReport();
uint256 reportRefSlot = refSlot + 1;

data = abi.encodePacked(
moduleId,
nodeOpId,
validatorIndex,
_validatorIndex,
validatorPubKey
);
IVEBO.ReportData memory report = IVEBO.ReportData({
Expand Down
2 changes: 1 addition & 1 deletion src/CSModule.sol
Original file line number Diff line number Diff line change
Expand Up @@ -541,12 +541,12 @@
function updateRefundedValidatorsCount(
uint256 /* nodeOperatorId */,
uint256 /* refundedValidatorsCount */
) external onlyRole(STAKING_ROUTER_ROLE) {
) external view onlyRole(STAKING_ROUTER_ROLE) {
revert NotSupported();
}

/// @inheritdoc IStakingModule
function updateTargetValidatorsLimits(

Check warning on line 549 in src/CSModule.sol

View workflow job for this annotation

GitHub Actions / Linters

Function order is incorrect, external function can not go after external view function (line 541)
uint256 nodeOperatorId,
uint256 targetLimitMode,
uint256 targetLimit
Expand Down
51 changes: 18 additions & 33 deletions test/BaseOracle.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -126,49 +126,45 @@ contract BaseOracleTest is Test, Utilities {
function test_submitConsensusReport_RevertsIfSenderIsNotConsensusContract()
public
{
uint256 initialRefSlot = oracle.getTime();
uint256 refSlot = oracle.getTime();
vm.prank(stranger);
vm.expectRevert(BaseOracle.SenderIsNotTheConsensusContract.selector);
oracle.submitConsensusReport(
keccak256("HASH_1"),
initialRefSlot,
initialRefSlot
);
oracle.submitConsensusReport(keccak256("HASH_1"), refSlot, refSlot);
}

function test_submitConsensusReport_SubmitsFromConsensusContract() public {
uint256 initialRefSlot = oracle.getTime();
uint256 refSlot = oracle.getTime();
vm.prank(address(consensus));
oracle.submitConsensusReport(
keccak256("HASH_1"),
initialRefSlot,
initialRefSlot + SLOTS_PER_FRAME
refSlot,
refSlot + SLOTS_PER_FRAME
);
assertEq(oracle.getConsensusReportLastCall().callCount, 1);
}

function test_discardConsensusReport_RevertsIfSenderIsNotConsensusContract()
public
{
uint256 initialRefSlot = oracle.getTime();
uint256 refSlot = oracle.getTime();
vm.prank(stranger);
vm.expectRevert(BaseOracle.SenderIsNotTheConsensusContract.selector);
oracle.discardConsensusReport(initialRefSlot);
oracle.discardConsensusReport(refSlot);
}

function test_discardConsensusReport_DiscardsFromConsensusContract()
public
{
uint256 initialRefSlot = oracle.getTime();
uint256 refSlot = oracle.getTime();
vm.prank(address(consensus));
oracle.submitConsensusReport(
keccak256("HASH_1"),
initialRefSlot,
initialRefSlot + SLOTS_PER_FRAME
refSlot,
refSlot + SLOTS_PER_FRAME
);
assertEq(oracle.getConsensusReportLastCall().callCount, 1);
vm.prank(address(consensus));
oracle.discardConsensusReport(initialRefSlot);
oracle.discardConsensusReport(refSlot);
}

// consensus tests
Expand Down Expand Up @@ -392,19 +388,19 @@ contract BaseOracleTest is Test, Utilities {
oracle.checkProcessingDeadline();
}

function test_isConsensusMember_ReturnsFalseOnNonMember() public {
function test_isConsensusMember_ReturnsFalseOnNonMember() public view {
bool r = oracle.isConsensusMember(notMember);
assertFalse(r);
}

function test_isConsensusMember_ReturnsTrueOnMember() public {
function test_isConsensusMember_ReturnsTrueOnMember() public view {
bool r = oracle.isConsensusMember(member);
assertTrue(r);
}

// consensus report

function test_getConsensusReport_ReturnsEmptyState() public {
function test_getConsensusReport_ReturnsEmptyState() public view {
(
bytes32 hash,
uint256 refSlot,
Expand Down Expand Up @@ -820,12 +816,7 @@ contract BaseOracleTest is Test, Utilities {
emit BaseOracle.ReportDiscarded(initialRefSlot, keccak256("HASH_1"));
oracle.discardConsensusReport(initialRefSlot);

(
bytes32 hash,
uint256 refSlot,
uint256 processingDeadlineTime,
bool processingStarted
) = oracle.getConsensusReport();
(bytes32 hash, , , ) = oracle.getConsensusReport();
assertEq(hash, bytes32(0));
}

Expand All @@ -842,8 +833,7 @@ contract BaseOracleTest is Test, Utilities {
vm.prank(address(consensus));
oracle.discardConsensusReport(initialRefSlot);

(bytes32 hash, uint256 refSlot, uint256 processingDeadlineTime) = oracle
.lastDiscardedReport();
(bytes32 hash, , ) = oracle.lastDiscardedReport();
assertEq(hash, keccak256("HASH_1"));
}

Expand All @@ -861,12 +851,7 @@ contract BaseOracleTest is Test, Utilities {
initialRefSlot,
deadline
);
(
bytes32 hash,
uint256 refSlot,
uint256 processingDeadlineTime,
bool processingStarted
) = oracle.getConsensusReport();
(bytes32 hash, , , ) = oracle.getConsensusReport();
assertEq(hash, keccak256("HASH_2"));
}

Expand Down Expand Up @@ -1099,7 +1084,7 @@ contract BaseOracleImpl is BaseOracle {
_checkConsensusData(refSlot, consensusVersion, hash);
}

function checkProcessingDeadline() external {
function checkProcessingDeadline() external view {
_checkProcessingDeadline(
_storageConsensusReport().value.processingDeadlineTime
);
Expand Down
Loading
Loading