Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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
74 changes: 4 additions & 70 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 @@ -2981,7 +2915,7 @@ contract ProposalValidator_SetProposalTypeData_Test is ProposalValidator_TestIni

/// @title ProposalValidator_HashProposalWithModule_Test
/// @notice Tests for the hashProposalWithModule function
contract ProposalValidator_HashProposalWithModule_Test is ProposalValidator_TestInit {
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];
}
}