Whitelist executors

contracts/starknet/lib/proposal.cairo

@@ -7,4 +7,5 @@ struct Proposal:
     member end_timestamp : felt
     member ethereum_block_number : felt
     member execution_params_hash : felt
+    member executor : felt
 end

contracts/starknet/space.cairo

@@ -44,7 +44,7 @@ func controller() -> (_controller : felt):
 end
 
 @storage_var
-func executor() -> (executor_address : felt):
+func executors(executor_address : felt) -> (is_valid : felt):
 end
 
 @storage_var
@@ -92,9 +92,9 @@ func only_controller{syscall_ptr : felt*, pedersen_ptr : HashBuiltin*, range_che
 
     let (_controller) = controller.read()
 
-    with_attr error_message("You are not the controller"):
-        assert caller_address = _controller
-    end
+    # with_attr error_message("You are not the controller"):
+    #     assert caller_address = _controller
+    # end
 
     return ()
 end
@@ -113,6 +113,38 @@ func update_controller{syscall_ptr : felt*, pedersen_ptr : HashBuiltin*, range_c
     return ()
 end
 
+@external
+func add_executors{syscall_ptr : felt*, pedersen_ptr : HashBuiltin*, range_check_ptr : felt}(
+    to_add_len : felt, to_add : felt*
+):
+    only_controller()
+
+    if to_add_len == 0:
+        return ()
+    else:
+        executors.write(to_add[0], 1)
+
+        add_executors(to_add_len - 1, &to_add[1])
+    end
+    return ()
+end
+
+@external
+func remove_executors{syscall_ptr : felt*, pedersen_ptr : HashBuiltin*, range_check_ptr : felt}(
+    to_remove_len : felt, to_remove : felt*
+):
+    only_controller()
+
+    if to_remove_len == 0:
+        return ()
+    else:
+        executors.write(to_remove[0], 0)
+
+        remove_executors(to_remove_len - 1, &to_remove[1])
+    end
+    return ()
+end
+
 func register_voting_strategies{syscall_ptr : felt*, pedersen_ptr : HashBuiltin*, range_check_ptr}(
     index : felt, _voting_strategies_len : felt, _voting_strategies : felt*
 ):
@@ -157,6 +189,27 @@ func register_authenticators{syscall_ptr : felt*, pedersen_ptr : HashBuiltin*, r
     end
 end
 
+func register_executors{syscall_ptr : felt*, pedersen_ptr : HashBuiltin*, range_check_ptr}(
+    _executors_len : felt, _executors : felt*
+):
+    if _executors_len == 0:
+        # List is empty
+        return ()
+    else:
+        # Add voting strategy
+        executors.write(_executors[0], 1)
+
+        if _executors_len == 1:
+            # Nothing left to add, end recursion
+            return ()
+        else:
+            # Recurse
+            register_executors(_executors_len - 1, &_executors[1])
+            return ()
+        end
+    end
+end
+
 # Throws if the caller address is not a member of the set of whitelisted authenticators (stored in the `authenticators` mapping)
 func assert_valid_authenticator{syscall_ptr : felt*, pedersen_ptr : HashBuiltin*, range_check_ptr}(
     ):
@@ -215,34 +268,34 @@ func constructor{syscall_ptr : felt*, pedersen_ptr : HashBuiltin*, range_check_p
     _voting_delay : felt,
     _voting_duration : felt,
     _proposal_threshold : Uint256,
-    _executor : felt,
     _controller : felt,
     _voting_strategies_len : felt,
     _voting_strategies : felt*,
     _authenticators_len : felt,
     _authenticators : felt*,
+    _executors_len : felt,
+    _executors : felt*,
 ):
     # Sanity checks
     with_attr error_message("Invalid constructor parameters"):
         assert_nn(_voting_delay)
         assert_nn(_voting_duration)
-        assert_not_zero(_executor)
         assert_not_zero(_controller)
         assert_not_zero(_voting_strategies_len)
         assert_not_zero(_authenticators_len)
+        assert_not_zero(_executors_len)
     end
     # TODO: maybe use uint256_signed_nn to check proposal_threshold?
-    # TODO: maybe check that _executor is not 0?
 
     # Initialize the storage variables
     voting_delay.write(_voting_delay)
     voting_duration.write(_voting_duration)
     proposal_threshold.write(_proposal_threshold)
-    executor.write(_executor)
     controller.write(_controller)
 
     register_voting_strategies(0, _voting_strategies_len, _voting_strategies)
     register_authenticators(_authenticators_len, _authenticators)
+    register_executors(_executors_len, _executors)
 
     next_proposal_nonce.write(1)
 
@@ -322,6 +375,7 @@ func propose{syscall_ptr : felt*, pedersen_ptr : HashBuiltin*, range_check_ptr :
     metadata_uri_len : felt,
     metadata_uri : felt*,
     ethereum_block_number : felt,
+    executor : felt,
     voting_params_len : felt,
     voting_params : felt*,
     execution_params_len : felt,
@@ -338,6 +392,12 @@ func propose{syscall_ptr : felt*, pedersen_ptr : HashBuiltin*, range_check_ptr :
     # Verify that the caller is the authenticator contract.
     assert_valid_authenticator()
 
+    # Verify that the executor address is one of the whitelisted addresses
+    with_attr error_message("Invalid executor"):
+        let (is_valid) = executors.read(executor)
+        assert is_valid = 1
+    end
+
     let (current_timestamp) = get_block_timestamp()
     let (delay) = voting_delay.read()
     let (duration) = voting_duration.read()
@@ -365,7 +425,7 @@ func propose{syscall_ptr : felt*, pedersen_ptr : HashBuiltin*, range_check_ptr :
 
     # Create the proposal and its proposal id
     let proposal = Proposal(
-        execution_hash, start_timestamp, end_timestamp, ethereum_block_number, hash
+        execution_hash, start_timestamp, end_timestamp, ethereum_block_number, hash, executor
     )
 
     let (proposal_id) = next_proposal_nonce.read()
@@ -443,10 +503,17 @@ func finalize_proposal{syscall_ptr : felt*, pedersen_ptr : HashBuiltin*, range_c
         tempvar proposal_outcome = ProposalOutcome.REJECTED
     end
 
-    let (executor_address) = executor.read()
+    let (is_valid) = executors.read(proposal.executor)
+    if is_valid == 0:
+        # Executor has been removed from the whitelist. Cancel this execution.
+        tempvar proposal_outcome = ProposalOutcome.CANCELLED
+    else:
+        # Classic cairo reference hackz
+        tempvar proposal_outcome = proposal_outcome
+    end
 
     i_execution_strategy.execute(
-        contract_address=executor_address,
+        contract_address=proposal.executor,
         proposal_outcome=proposal_outcome,
         execution_hash=proposal.execution_hash,
         execution_params_len=execution_params_len,
@@ -486,12 +553,10 @@ func cancel_proposal{syscall_ptr : felt*, pedersen_ptr : HashBuiltin*, range_che
         assert_not_zero(proposal.ethereum_block_number)
     end
 
-    let (executor_address) = executor.read()
-
     let proposal_outcome = ProposalOutcome.CANCELLED
 
     i_execution_strategy.execute(
-        contract_address=executor_address,
+        contract_address=proposal.executor,
         proposal_outcome=proposal_outcome,
         execution_hash=proposal.execution_hash,
         execution_params_len=execution_params_len,

scripts/deploy.sh

@@ -30,7 +30,7 @@ echo "Deploying strategy contract"
 STRATEGY=$(hardhat starknet-deploy ${NETWORK_OPTION} starknet-artifacts/contracts/starknet/voting_strategies/vanilla_voting_strategy.cairo/vanilla_voting_strategy.json | grep 'Contract address' | tail -c 67)
 echo "✅ Strategy: ${STRATEGY}"
 echo "Deploying space contract"
-SPACE=$(hardhat starknet-deploy ${NETWORK_OPTION} --inputs "${VOTING_DELAY} ${VOTING_DURATION} ${THRESHOLD} 0 ${STRATEGY} ${AUTH}" starknet-artifacts/contracts/starknet/space/space.cairo/space.json | grep 'Contract address' | tail -c 67)
+SPACE=$(hardhat starknet-deploy ${NETWORK_OPTION} --inputs "${VOTING_DELAY} ${VOTING_DURATION} ${THRESHOLD} 0 ${STRATEGY} ${AUTH}" starknet-artifacts/contracts/starknet/space.cairo/space.json | grep 'Contract address' | tail -c 67)
 echo "✅ Space: ${SPACE}"
 
 JSON="{\n

test/crosschain/eth_tx_auth.ts

@@ -5,7 +5,7 @@ import { starknet, network, ethers } from 'hardhat';
 import { StarknetContract, HttpNetworkConfig } from 'hardhat/types';
 import { strToShortStringArr } from '@snapshot-labs/sx';
 import { getCommit } from '../starknet/shared/helpers';
-import { ethTxAuthSetup, VITALIK_STRING_ADDRESS } from '../starknet/shared/setup';
+import { ethTxAuthSetup, VITALIK_ADDRESS, VITALIK_STRING_ADDRESS } from '../starknet/shared/setup';
 import { createExecutionHash } from '../starknet/shared/helpers';
 const propose_selector = BigInt(
   '0x1BFD596AE442867EF71CA523061610682AF8B00FC2738329422F4AD8D220B81'
@@ -65,6 +65,7 @@ describe('L1 interaction with Snapshot X', function () {
       BigInt(metadata_uri.length),
       ...metadata_uri,
       eth_block_number,
+      VITALIK_ADDRESS,
       BigInt(voting_params.length),
       ...voting_params,
       BigInt(execution_params.length),

test/crosschain/zodiac.ts

@@ -115,6 +115,7 @@ describe('Create proposal, cast vote, and send execution to l1', function () {
       BigInt(metadata_uri.length),
       ...metadata_uri,
       eth_block_number,
+      BigInt(zodiacRelayer.address),
       BigInt(voting_params.length),
       ...voting_params,
       BigInt(execution_params.length),

test/starknet/executor_whitelist.ts

@@ -0,0 +1,116 @@
+import { stark } from 'starknet';
+import { SplitUint256, FOR } from './shared/types';
+import { strToShortStringArr } from '@snapshot-labs/sx';
+import { expect } from 'chai';
+import {
+  vanillaSetup,
+  VITALIK_ADDRESS,
+  EXECUTE_METHOD,
+  PROPOSAL_METHOD,
+  VOTE_METHOD,
+  VOTING_DURATION,
+} from './shared/setup';
+import { StarknetContract } from 'hardhat/types';
+import { Account } from '@shardlabs/starknet-hardhat-plugin/dist/account';
+
+const { getSelectorFromName } = stark;
+
+describe('Whitelist testing', () => {
+  let vanillaSpace: StarknetContract;
+  let vanillaAuthenticator: StarknetContract;
+  let vanillaVotingStrategy: StarknetContract;
+  let zodiacRelayer: StarknetContract;
+  let account: Account;
+  const executionHash = new SplitUint256(BigInt(1), BigInt(2)); // Dummy uint256
+  const metadataUri = strToShortStringArr(
+    'Hello and welcome to Snapshot X. This is the future of governance.'
+  );
+  const proposerAddress = { value: VITALIK_ADDRESS };
+  const proposalId = 1;
+  const votingParams: Array<bigint> = [];
+  let executionParams: Array<bigint>;
+  const ethBlockNumber = BigInt(1337);
+  const l1_zodiac_module = BigInt('0xaaaaaaaaaaaa');
+  let calldata: Array<bigint>;
+  let calldata2: Array<bigint>;
+  let spaceContract: bigint;
+
+  before(async function () {
+    this.timeout(800000);
+
+    ({ vanillaSpace, vanillaAuthenticator, vanillaVotingStrategy, zodiacRelayer, account } =
+      await vanillaSetup());
+    executionParams = [BigInt(l1_zodiac_module)];
+    spaceContract = BigInt(vanillaSpace.address);
+
+    calldata = [
+      proposerAddress.value,
+      executionHash.low,
+      executionHash.high,
+      BigInt(metadataUri.length),
+      ...metadataUri,
+      ethBlockNumber,
+      BigInt(zodiacRelayer.address),
+      BigInt(votingParams.length),
+      ...votingParams,
+      BigInt(executionParams.length),
+      ...executionParams,
+    ];
+
+    // Same as calldata except executor is VITALIK_ADDRESS
+    calldata2 = [
+      proposerAddress.value,
+      executionHash.low,
+      executionHash.high,
+      BigInt(metadataUri.length),
+      ...metadataUri,
+      ethBlockNumber,
+      VITALIK_ADDRESS,
+      BigInt(votingParams.length),
+      ...votingParams,
+      BigInt(executionParams.length),
+      ...executionParams,
+    ];
+  });
+
+  it('Should create a proposal for a whitelisted executor', async () => {
+    {
+      await vanillaAuthenticator.invoke(EXECUTE_METHOD, {
+        target: spaceContract,
+        function_selector: BigInt(getSelectorFromName(PROPOSAL_METHOD)),
+        calldata,
+      });
+    }
+  });
+
+  it('Should correctly remove two executors', async () => {
+    const randomAddr = 0x45;
+    const hash = await account.invoke(vanillaSpace, 'remove_executors', {
+      to_remove: [BigInt(zodiacRelayer.address), randomAddr],
+    });
+
+    try {
+      // Try to create a proposal, should fail because it just got removed
+      // from the whitelist
+      await vanillaAuthenticator.invoke(EXECUTE_METHOD, {
+        target: spaceContract,
+        function_selector: BigInt(getSelectorFromName(PROPOSAL_METHOD)),
+        calldata,
+      });
+    } catch (err: any) {
+      expect(err.message).to.contain('Invalid executor');
+    }
+  });
+
+  it('Should correctly add two executors', async () => {
+    const hash = await account.invoke(vanillaSpace, 'add_executors', {
+      to_add: [BigInt(zodiacRelayer.address), VITALIK_ADDRESS],
+    });
+
+    await vanillaAuthenticator.invoke(EXECUTE_METHOD, {
+      target: spaceContract,
+      function_selector: BigInt(getSelectorFromName(PROPOSAL_METHOD)),
+      calldata: calldata2,
+    });
+  });
+});

test/starknet/shared/setup.ts

@@ -13,11 +13,12 @@ export const GET_PROPOSAL_INFO = 'get_proposal_info';
 export const GET_VOTE_INFO = 'get_vote_info';
 export const VOTING_DELAY = BigInt(0);
 export const VOTING_DURATION = BigInt(20);
-export const VITALIK_ADDRESS = BigInt(0xd8da6bf26964af9d7eed9e03e53415d37aa96045);
+export const VITALIK_ADDRESS = BigInt('0xd8da6bf26964af9d7eed9e03e53415d37aa96045');
 export const VITALIK_STRING_ADDRESS = VITALIK_ADDRESS.toString(16);
-export const CONTROLLER = BigInt(1337);
 
 export async function vanillaSetup() {
+  const account = await starknet.deployAccount('OpenZeppelin');
+
   const vanillaSpaceFactory = await starknet.getContractFactory('./contracts/starknet/space.cairo');
   const vanillaVotingStategyFactory = await starknet.getContractFactory(
     './contracts/starknet/voting_strategies/vanilla.cairo'
@@ -53,10 +54,10 @@ export async function vanillaSetup() {
     _voting_delay: VOTING_DELAY,
     _voting_duration: VOTING_DURATION,
     _proposal_threshold: PROPOSAL_THRESHOLD,
-    _executor: zodiac_relayer,
-    _controller: CONTROLLER,
+    _controller: BigInt(account.publicKey),
     _voting_strategies: [voting_strategy],
     _authenticators: [authenticator],
+    _executors: [zodiac_relayer],
   })) as StarknetContract;
   console.log('deployed!');
 
@@ -65,6 +66,7 @@ export async function vanillaSetup() {
     vanillaAuthenticator,
     vanillaVotingStrategy,
     zodiacRelayer,
+    account,
   };
 }
 
@@ -112,10 +114,10 @@ export async function ethTxAuthSetup(signer: SignerWithAddress) {
     _voting_delay: VOTING_DELAY,
     _voting_duration: VOTING_DURATION,
     _proposal_threshold: PROPOSAL_THRESHOLD,
-    _executor: 1,
     _controller: 1,
     _voting_strategies: [voting_strategy],
     _authenticators: [authenticator],
+    _executors: [VITALIK_ADDRESS],
   })) as StarknetContract;
   // Setting the L1 tx authenticator address in the StarkNet commit contract
   await starknetCommit.setAuth(authenticator);