Skip to content
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 packages/contracts-bedrock/snapshots/semver-lock.json
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@
},
"src/governance/ProposalValidator.sol:ProposalValidator": {
"initCodeHash": "0x3d2dc05bda4ddd8412b16da644ef8d2cbffb0b93c9ba326639ae73c7eedd890d",
"sourceCodeHash": "0x23e62fcea15b38d3e121d8d880ed097b01eefcd7343607bf74ae9f20b620b8b6"
"sourceCodeHash": "0x6f62e5285eb2d328f053d168dd7312d16b1557f9e9a08025c18f1916367c4b26"
},
"src/legacy/DeployerWhitelist.sol:DeployerWhitelist": {
"initCodeHash": "0x53099379ed48b87f027d55712dbdd1da7d7099925426eb0531da9c0012e02c29",
Expand Down
79 changes: 7 additions & 72 deletions packages/contracts-bedrock/test/governance/ProposalValidator.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -23,82 +23,16 @@ import { stdStorage, StdStorage } from "forge-std/Test.sol";
// Contracts
import { ProposalValidator } from "src/governance/ProposalValidator.sol";

// Mocks
import { ProposalValidatorForTest } from "test/mocks/ProposalValidatorForTest.sol";

// Libraries
import { Predeploys } from "src/libraries/Predeploys.sol";

// Testing utilities
import { stdStorage, StdStorage } from "forge-std/Test.sol";
import { CommonTest } from "test/setup/CommonTest.sol";

/// @title ProposalValidatorForTest
/// @notice A test contract that exposes the private _hashProposalWithModule function
contract ProposalValidatorForTest is ProposalValidator {
constructor(
address _owner,
IOptimismGovernor _governor,
bytes32 _approvedProposerAttestationSchemaUid,
bytes32 _topDelegatesAttestationSchemaUid
)
ProposalValidator(_owner, _governor, _approvedProposerAttestationSchemaUid, _topDelegatesAttestationSchemaUid)
{ }

function hashProposalWithModule(
address _module,
bytes memory _proposalData,
bytes32 _descriptionHash
)
public
view
returns (uint256)
{
return _hashProposalWithModule(_module, _proposalData, _descriptionHash);
}

/// @notice Exposes proposal data for testing
function getProposalData(uint256 _proposalId)
public
view
returns (
address proposer_,
ProposalType proposalType_,
bool movedToVote_,
uint256 approvalCount_,
uint256 votingCycle_
)
{
ProposalData storage proposal = _proposals[_proposalId];
return (
proposal.proposer, proposal.proposalType, proposal.movedToVote, proposal.approvalCount, proposal.votingCycle
);
}

function setProposalData(
uint256 _proposalId,
address _proposer,
ProposalType _proposalType,
bool _movedToVote,
uint256 _approvalCount,
uint256 _votingCycle
)
public
{
_proposals[_proposalId].proposer = _proposer;
_proposals[_proposalId].proposalType = _proposalType;
_proposals[_proposalId].movedToVote = _movedToVote;
_proposals[_proposalId].approvalCount = _approvalCount;
_proposals[_proposalId].votingCycle = _votingCycle;
}

function mockApproveProposal(uint256 _proposalId, address _delegate) public {
_proposals[_proposalId].delegateApprovals[_delegate] = true;
}

/// @notice Check if a delegate has approved a proposal
function hasDelegateApproved(uint256 _proposalId, address _delegate) public view returns (bool hasApproved_) {
return _proposals[_proposalId].delegateApprovals[_delegate];
}
}

/// @title ProposalValidator_TestInit
/// @notice Setup contract for ProposalValidator tests
contract ProposalValidator_TestInit is CommonTest {
Expand Down Expand Up @@ -2979,9 +2913,10 @@ contract ProposalValidator_SetProposalTypeData_Test is ProposalValidator_TestIni
}
}

/// @title ProposalValidator_HashProposalWithModule_Test
/// @notice Tests for the hashProposalWithModule function
contract ProposalValidator_HashProposalWithModule_Test is ProposalValidator_TestInit {
/// @title ProposalValidator_Uncategorized_Test
/// @notice Tests for the `_hashProposalWithModule` function that is not part of the public interface
/// @dev This internal function is only exposed through the ProposalValidatorForTest contract
contract ProposalValidator_Uncategorized_Test is ProposalValidator_TestInit {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HashProposalWithModule is not a function right?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is, but it's internal.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok lets add a comment on why the Uncategorized so its clear what we are testing

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in here.

function testFuzz_hashProposalWithModule_succeeds(
address fuzzedModule,
bytes memory fuzzedProposalData,
Expand Down
77 changes: 77 additions & 0 deletions packages/contracts-bedrock/test/mocks/ProposalValidatorForTest.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.15;

// Interfaces
import { IOptimismGovernor } from "interfaces/governance/IOptimismGovernor.sol";

// Contracts
import { ProposalValidator } from "src/governance/ProposalValidator.sol";

/// @title ProposalValidatorForTest
/// @notice A test contract that exposes the private _hashProposalWithModule function
contract ProposalValidatorForTest is ProposalValidator {
constructor(
address _owner,
IOptimismGovernor _governor,
bytes32 _approvedProposerAttestationSchemaUid,
bytes32 _topDelegatesAttestationSchemaUid
)
ProposalValidator(_owner, _governor, _approvedProposerAttestationSchemaUid, _topDelegatesAttestationSchemaUid)
{ }

function hashProposalWithModule(
address _module,
bytes memory _proposalData,
bytes32 _descriptionHash
)
public
view
returns (uint256)
{
return _hashProposalWithModule(_module, _proposalData, _descriptionHash);
}

/// @notice Exposes proposal data for testing
function getProposalData(uint256 _proposalId)
public
view
returns (
address proposer_,
ProposalType proposalType_,
bool movedToVote_,
uint256 approvalCount_,
uint256 votingCycle_
)
{
ProposalData storage proposal = _proposals[_proposalId];
return (
proposal.proposer, proposal.proposalType, proposal.movedToVote, proposal.approvalCount, proposal.votingCycle
);
}

function setProposalData(
uint256 _proposalId,
address _proposer,
ProposalType _proposalType,
bool _movedToVote,
uint256 _approvalCount,
uint256 _votingCycle
)
public
{
_proposals[_proposalId].proposer = _proposer;
_proposals[_proposalId].proposalType = _proposalType;
_proposals[_proposalId].movedToVote = _movedToVote;
_proposals[_proposalId].approvalCount = _approvalCount;
_proposals[_proposalId].votingCycle = _votingCycle;
}

function mockApproveProposal(uint256 _proposalId, address _delegate) public {
_proposals[_proposalId].delegateApprovals[_delegate] = true;
}

/// @notice Check if a delegate has approved a proposal
function hasDelegateApproved(uint256 _proposalId, address _delegate) public view returns (bool hasApproved_) {
return _proposals[_proposalId].delegateApprovals[_delegate];
}
}