Skip to content

Commit

Permalink
chore: address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
LHerskind committed Jan 3, 2025
1 parent 5c70d4d commit 8000d59
Show file tree
Hide file tree
Showing 13 changed files with 120 additions and 119 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ import {IPayload} from "@aztec/governance/interfaces/IPayload.sol";

interface IGovernanceProposer {
event VoteCast(IPayload indexed proposal, uint256 indexed round, address indexed voter);
event ProposalPushed(IPayload indexed proposal, uint256 indexed round);
event ProposalExecuted(IPayload indexed proposal, uint256 indexed round);

function vote(IPayload _proposal) external returns (bool);
function pushProposal(uint256 _roundNumber) external returns (bool);
function executeProposal(uint256 _roundNumber) external returns (bool);
function yeaCount(address _instance, uint256 _round, IPayload _proposal)
external
view
Expand Down
2 changes: 1 addition & 1 deletion l1-contracts/src/governance/libraries/Errors.sol
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ library Errors {
error Governance__ProposalLib__ZeroYeaVotesNeeded();
error Governance__ProposalLib__MoreYeaVoteThanExistNeeded();

error GovernanceProposer__CanOnlyPushProposalInPast(); // 0x84a5b5ae
error GovernanceProposer__CanOnlyExecuteProposalInPast(); // 0x8bf1d3b8
error GovernanceProposer__FailedToPropose(IPayload proposal); // 0x8d94fbfc
error GovernanceProposer__InstanceHaveNoCode(address instance); // 0x5fa92625
error GovernanceProposer__InsufficientVotes(uint256 votesCast, uint256 votesNeeded); // 0xd4ad89c2
Expand Down
12 changes: 8 additions & 4 deletions l1-contracts/src/governance/proposer/EmpireBase.sol
Original file line number Diff line number Diff line change
Expand Up @@ -91,13 +91,17 @@ abstract contract EmpireBase is IGovernanceProposer {
}

/**
* @notice Push the proposal to the appela
* @notice Executes the proposal using the `_execute` function
*
* @param _roundNumber - The round number to execute
*
* @return True if executed successfully, false otherwise
*/
function pushProposal(uint256 _roundNumber) external override(IGovernanceProposer) returns (bool) {
function executeProposal(uint256 _roundNumber)
external
override(IGovernanceProposer)
returns (bool)
{
// Need to ensure that the round is not active.
address instance = getInstance();
require(instance.code.length > 0, Errors.GovernanceProposer__InstanceHaveNoCode(instance));
Expand All @@ -106,7 +110,7 @@ abstract contract EmpireBase is IGovernanceProposer {
Slot currentSlot = selection.getCurrentSlot();

uint256 currentRound = computeRound(currentSlot);
require(_roundNumber < currentRound, Errors.GovernanceProposer__CanOnlyPushProposalInPast());
require(_roundNumber < currentRound, Errors.GovernanceProposer__CanOnlyExecuteProposalInPast());
require(
_roundNumber + LIFETIME_IN_ROUNDS >= currentRound,
Errors.GovernanceProposer__ProposalTooOld(_roundNumber, currentRound)
Expand All @@ -122,7 +126,7 @@ abstract contract EmpireBase is IGovernanceProposer {

round.executed = true;

emit ProposalPushed(round.leader, _roundNumber);
emit ProposalExecuted(round.leader, _roundNumber);

require(_execute(round.leader), Errors.GovernanceProposer__FailedToPropose(round.leader));
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {Slot, SlotLib, Timestamp} from "@aztec/core/libraries/TimeMath.sol";
import {FaultyGovernance} from "./mocks/FaultyGovernance.sol";
import {FalsyGovernance} from "./mocks/FalsyGovernance.sol";

contract PushProposalTest is GovernanceProposerBase {
contract ExecuteProposalTest is GovernanceProposerBase {
using SlotLib for Slot;

Leonidas internal leonidas;
Expand All @@ -26,7 +26,7 @@ contract PushProposalTest is GovernanceProposerBase {
Errors.GovernanceProposer__InstanceHaveNoCode.selector, address(0xdead)
)
);
governanceProposer.pushProposal(_roundNumber);
governanceProposer.executeProposal(_roundNumber);
}

modifier givenCanonicalInstanceHoldCode() {
Expand All @@ -42,9 +42,9 @@ contract PushProposalTest is GovernanceProposerBase {
function test_WhenRoundNotInPast() external givenCanonicalInstanceHoldCode {
// it revert
vm.expectRevert(
abi.encodeWithSelector(Errors.GovernanceProposer__CanOnlyPushProposalInPast.selector)
abi.encodeWithSelector(Errors.GovernanceProposer__CanOnlyExecuteProposalInPast.selector)
);
governanceProposer.pushProposal(0);
governanceProposer.executeProposal(0);
}

modifier whenRoundInPast() {
Expand Down Expand Up @@ -74,7 +74,7 @@ contract PushProposalTest is GovernanceProposerBase {
governanceProposer.computeRound(leonidas.getCurrentSlot())
)
);
governanceProposer.pushProposal(0);
governanceProposer.executeProposal(0);
}

modifier whenRoundInRecentPast() {
Expand Down Expand Up @@ -105,13 +105,13 @@ contract PushProposalTest is GovernanceProposerBase {
)
)
);
governanceProposer.pushProposal(1);
governanceProposer.executeProposal(1);
}

vm.expectRevert(
abi.encodeWithSelector(Errors.GovernanceProposer__ProposalAlreadyExecuted.selector, 1)
);
governanceProposer.pushProposal(1);
governanceProposer.executeProposal(1);
}

modifier givenRoundNotExecutedBefore() {
Expand Down Expand Up @@ -144,7 +144,7 @@ contract PushProposalTest is GovernanceProposerBase {
vm.expectRevert(
abi.encodeWithSelector(Errors.GovernanceProposer__ProposalCannotBeAddressZero.selector)
);
governanceProposer.pushProposal(0);
governanceProposer.executeProposal(0);
}

modifier givenLeaderIsNotAddress0() {
Expand Down Expand Up @@ -174,7 +174,7 @@ contract PushProposalTest is GovernanceProposerBase {
vm.expectRevert(
abi.encodeWithSelector(Errors.GovernanceProposer__InsufficientVotes.selector, 1, votesNeeded)
);
governanceProposer.pushProposal(1);
governanceProposer.executeProposal(1);
}

modifier givenSufficientYea(uint256 _yeas) {
Expand Down Expand Up @@ -219,9 +219,9 @@ contract PushProposalTest is GovernanceProposerBase {

// As time is perceived differently, round 1 is currently in the future
vm.expectRevert(
abi.encodeWithSelector(Errors.GovernanceProposer__CanOnlyPushProposalInPast.selector)
abi.encodeWithSelector(Errors.GovernanceProposer__CanOnlyExecuteProposalInPast.selector)
);
governanceProposer.pushProposal(1);
governanceProposer.executeProposal(1);

// Jump 2 rounds, since we are currently in round 0
vm.warp(
Expand All @@ -234,7 +234,7 @@ contract PushProposalTest is GovernanceProposerBase {
vm.expectRevert(
abi.encodeWithSelector(Errors.GovernanceProposer__ProposalCannotBeAddressZero.selector)
);
governanceProposer.pushProposal(1);
governanceProposer.executeProposal(1);
}

function test_GivenGovernanceCallReturnFalse(uint256 _yeas)
Expand All @@ -253,7 +253,7 @@ contract PushProposalTest is GovernanceProposerBase {
vm.expectRevert(
abi.encodeWithSelector(Errors.GovernanceProposer__FailedToPropose.selector, proposal)
);
governanceProposer.pushProposal(1);
governanceProposer.executeProposal(1);
}

function test_GivenGovernanceCallFails(uint256 _yeas)
Expand All @@ -270,7 +270,7 @@ contract PushProposalTest is GovernanceProposerBase {
vm.etch(address(governance), address(faulty).code);

vm.expectRevert(abi.encodeWithSelector(FaultyGovernance.Faulty.selector));
governanceProposer.pushProposal(1);
governanceProposer.executeProposal(1);
}

function test_GivenGovernanceCallSucceeds(uint256 _yeas)
Expand All @@ -283,11 +283,11 @@ contract PushProposalTest is GovernanceProposerBase {
givenSufficientYea(_yeas)
{
// it update executed to true
// it emits {ProposalPushed} event
// it emits {ProposalExecuted} event
// it return true
vm.expectEmit(true, true, true, true, address(governanceProposer));
emit IGovernanceProposer.ProposalPushed(proposal, 1);
assertTrue(governanceProposer.pushProposal(1));
emit IGovernanceProposer.ProposalExecuted(proposal, 1);
assertTrue(governanceProposer.executeProposal(1));
(, IPayload leader, bool executed) = governanceProposer.rounds(address(leonidas), 1);
assertTrue(executed);
assertEq(address(leader), address(proposal));
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
PushProposalTest
ExecuteProposalTest
├── given canonical instance hold no code
│ └── it revert
└── given canonical instance hold code
Expand All @@ -25,5 +25,5 @@ PushProposalTest
│ └── it revert
└── given governance call succeeds
├── it update executed to true
├── it emits {ProposalPushed} event
├── it emits {ProposalExecuted} event
└── it return true
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ contract UpgradeGovernanceProposerTest is TestBase {
vm.warp(Timestamp.unwrap(rollup.getTimestampForSlot(rollup.getCurrentSlot() + Slot.wrap(1))));
}

governanceProposer.pushProposal(0);
governanceProposer.executeProposal(0);
proposal = governance.getProposal(0);
assertEq(address(proposal.payload), address(payload));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ contract SlashingScenario is TestBase {
assertTrue(info.status == Status.VALIDATING, "Invalid status");
}

slashingProposer.pushProposal(0);
slashingProposer.executeProposal(0);

// Make sure that the slash was successful,
// Meaning that validators are now LIVING and have lost the slash amount
Expand Down
26 changes: 20 additions & 6 deletions yarn-project/end-to-end/src/e2e_p2p/p2p_network.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { getSchnorrAccount } from '@aztec/accounts/schnorr';
import { type AztecNodeConfig, type AztecNodeService } from '@aztec/aztec-node';
import { type AccountWalletWithSecretKey } from '@aztec/aztec.js';
import { getL1ContractsConfigEnvVars } from '@aztec/ethereum';
import { L1TxUtils, getL1ContractsConfigEnvVars } from '@aztec/ethereum';
import { EthCheatCodesWithState } from '@aztec/ethereum/test';
import { type Logger, createLogger } from '@aztec/foundation/log';
import { RollupAbi, TestERC20Abi } from '@aztec/l1-artifacts';
Expand Down Expand Up @@ -53,6 +53,8 @@ export class P2PNetworkTest {

private cleanupInterval: NodeJS.Timeout | undefined = undefined;

private gasUtils: L1TxUtils | undefined = undefined;

constructor(
testName: string,
public bootstrapNode: BootstrapNode,
Expand Down Expand Up @@ -146,15 +148,13 @@ export class P2PNetworkTest {
this.logger.info('Syncing mock system time');
const { dateProvider, deployL1ContractsValues } = this.ctx!;
// Send a tx and only update the time after the tx is mined, as eth time is not continuous
const tx = await deployL1ContractsValues.walletClient.sendTransaction({
const receipt = await this.gasUtils!.sendAndMonitorTransaction({
to: this.baseAccount.address,
data: '0x',
value: 1n,
account: this.baseAccount,
});
const receipt = await deployL1ContractsValues.publicClient.waitForTransactionReceipt({
hash: tx,
});
const timestamp = await deployL1ContractsValues.publicClient.getBlock({ blockNumber: receipt.blockNumber });
this.logger.info(`Timestamp: ${timestamp.timestamp}`);
dateProvider.setTime(Number(timestamp.timestamp) * 1000);
}

Expand Down Expand Up @@ -294,6 +294,20 @@ export class P2PNetworkTest {
async setup() {
this.ctx = await this.snapshotManager.setup();
this.startSyncMockSystemTimeInterval();

this.gasUtils = new L1TxUtils(
this.ctx.deployL1ContractsValues.publicClient,
this.ctx.deployL1ContractsValues.walletClient,
this.logger,
{
gasLimitBufferPercentage: 20n,
maxGwei: 500n,
minGwei: 1n,
maxAttempts: 3,
checkIntervalMs: 100,
stallTimeMs: 1000,
},
);
}

async stopNodes(nodes: AztecNodeService[]) {
Expand Down
Loading

0 comments on commit 8000d59

Please sign in to comment.