diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 6d661804..7e068495 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -4,6 +4,7 @@ env: STARKNET_SIERRA_COMPILE_PATH: ./cairo/bin/starknet-sierra-compile OBJC_DISABLE_INITIALIZE_FORK_SAFETY: YES ADDRESS: "0x347be35996a21f6bf0623e75dbce52baba918ad5ae8d83b6f416045ab22961a" + PUBLIC_KEY: "0x674efe292c3c1125108916d6128bd6d1db4528db07322a84177551067aa2bef" PK: "0xbdd640fb06671ad11c80317fa3b1799d" on: diff --git a/starknet/Scarb.toml b/starknet/Scarb.toml index 5f32ebd1..2c0ebe15 100644 --- a/starknet/Scarb.toml +++ b/starknet/Scarb.toml @@ -9,6 +9,7 @@ allowed-libfuncs-list.name = "audited" sierra = true casm = true casm-add-pythonic-hints = true +build-external-contracts = ["openzeppelin::account::account::Account", "openzeppelin::account::interface::AccountABI"] [dependencies] openzeppelin = { git = "https://github.com/ericnordelo/cairo-contracts", branch = "feat/erc20votes-#631" } diff --git a/starknet/src/authenticators/stark_sig.cairo b/starknet/src/authenticators/stark_sig.cairo index 42f31715..e6008068 100644 --- a/starknet/src/authenticators/stark_sig.cairo +++ b/starknet/src/authenticators/stark_sig.cairo @@ -4,6 +4,7 @@ use sx::types::{Strategy, IndexedStrategy, Choice}; #[starknet::interface] trait IStarkSigAuthenticator { /// Authenticates a propose transaction using the starknet EIP712-equivalent signature scheme. + /// Note: Only SNIP-6 compliant accounts are supported. /// /// # Arguments /// @@ -14,7 +15,6 @@ trait IStarkSigAuthenticator { /// * `execution_strategy` - The execution strategy of the proposal. /// * `user_proposal_validation_params` - The user proposal validation params of the proposal. /// * `salt` - The salt, used for replay protection. - /// * `account_type` - The account type of the author ('snake' or 'camel'). fn authenticate_propose( ref self: TContractState, signature: Array, @@ -23,13 +23,13 @@ trait IStarkSigAuthenticator { metadata_uri: Array, execution_strategy: Strategy, user_proposal_validation_params: Array, - salt: felt252, - account_type: felt252 + salt: felt252 ); /// Authenticates a vote transaction using the starknet EIP712-equivalent signature scheme. - /// Salt is not needed because double voting is prevented by the space itself. + /// Note: Salt is not needed because double voting is prevented by the space itself. + /// Note: Only SNIP-6 compliant accounts are supported. /// /// # Arguments /// @@ -40,8 +40,6 @@ trait IStarkSigAuthenticator { /// * `choice` - The choice of the voter. /// * `user_voting_strategies` - The user voting strategies of the voter. /// * `metadata_uri` - The URI of the proposal metadata. - /// * `account_type` - The account type of the voter ('snake' or 'camel'). - /// fn authenticate_vote( ref self: TContractState, signature: Array, @@ -50,11 +48,11 @@ trait IStarkSigAuthenticator { proposal_id: u256, choice: Choice, user_voting_strategies: Array, - metadata_uri: Array, - account_type: felt252 + metadata_uri: Array ); /// Authenticates an update proposal transaction using the starknet EIP712-equivalent signature scheme. + /// Note: Only SNIP-6 compliant accounts are supported. /// /// # Arguments /// @@ -65,7 +63,6 @@ trait IStarkSigAuthenticator { /// * `execution_strategy` - The execution strategy of the proposal. /// * `metadata_uri` - The URI of the proposal metadata. /// * `salt` - The salt, used for replay protection. - /// * `account_type` - The account type of the author ('snake' or 'camel'). fn authenticate_update_proposal( ref self: TContractState, signature: Array, @@ -74,8 +71,7 @@ trait IStarkSigAuthenticator { proposal_id: u256, execution_strategy: Strategy, metadata_uri: Array, - salt: felt252, - account_type: felt252 + salt: felt252 ); } @@ -103,8 +99,7 @@ mod StarkSigAuthenticator { metadata_uri: Array, execution_strategy: Strategy, user_proposal_validation_params: Array, - salt: felt252, - account_type: felt252 + salt: felt252 ) { assert(!self._used_salts.read((author, salt)), 'Salt Already Used'); @@ -117,8 +112,7 @@ mod StarkSigAuthenticator { metadata_uri.span(), @execution_strategy, user_proposal_validation_params.span(), - salt, - account_type + salt ); self._used_salts.write((author, salt), true); @@ -139,8 +133,7 @@ mod StarkSigAuthenticator { proposal_id: u256, choice: Choice, user_voting_strategies: Array, - metadata_uri: Array, - account_type: felt252 + metadata_uri: Array ) { // No need to check salts here, as double voting is prevented by the space itself. @@ -153,8 +146,7 @@ mod StarkSigAuthenticator { proposal_id, choice, user_voting_strategies.span(), - metadata_uri.span(), - account_type + metadata_uri.span() ); ISpaceDispatcher { contract_address: target } @@ -175,8 +167,7 @@ mod StarkSigAuthenticator { proposal_id: u256, execution_strategy: Strategy, metadata_uri: Array, - salt: felt252, - account_type: felt252 + salt: felt252 ) { assert(!self._used_salts.read((author, salt)), 'Salt Already Used'); @@ -189,8 +180,7 @@ mod StarkSigAuthenticator { proposal_id, @execution_strategy, metadata_uri.span(), - salt, - account_type + salt ); self._used_salts.write((author, salt), true); diff --git a/starknet/src/interfaces/i_account.cairo b/starknet/src/interfaces/i_account.cairo deleted file mode 100644 index bfa9fc1a..00000000 --- a/starknet/src/interfaces/i_account.cairo +++ /dev/null @@ -1,32 +0,0 @@ -use starknet::account::Call; -use starknet::ContractAddress; - -// Interfaces from OZ: https://github.com/OpenZeppelin/cairo-contracts/blob/cairo-2/src/account/interface.cairo -#[starknet::interface] -trait AccountABI { - fn __execute__(self: @TState, calls: Array) -> Array>; - fn __validate__(self: @TState, calls: Array) -> felt252; - fn __validate_declare__(self: @TState, class_hash: felt252) -> felt252; - fn __validate_deploy__( - self: @TState, class_hash: felt252, contract_address_salt: felt252, _public_key: felt252 - ) -> felt252; - fn set_public_key(ref self: TState, new_public_key: felt252); - fn get_public_key(self: @TState) -> felt252; - fn is_valid_signature(self: @TState, hash: felt252, signature: Array) -> felt252; - fn supports_interface(self: @TState, interface_id: felt252) -> bool; -} - -// Entry points case-convention is enforced by the protocol -#[starknet::interface] -trait AccountCamelABI { - fn __execute__(self: @TState, calls: Array) -> Array>; - fn __validate__(self: @TState, calls: Array) -> felt252; - fn __validate_declare__(self: @TState, classHash: felt252) -> felt252; - fn __validate_deploy__( - self: @TState, classHash: felt252, contractAddressSalt: felt252, _publicKey: felt252 - ) -> felt252; - fn setPublicKey(ref self: TState, newPublicKey: felt252); - fn getPublicKey(self: @TState) -> felt252; - fn isValidSignature(self: @TState, hash: felt252, signature: Array) -> felt252; - fn supportsInterface(self: @TState, interfaceId: felt252) -> bool; -} diff --git a/starknet/src/lib.cairo b/starknet/src/lib.cairo index 4bd0942f..c0aee34c 100644 --- a/starknet/src/lib.cairo +++ b/starknet/src/lib.cairo @@ -46,12 +46,6 @@ mod factory { } mod interfaces { - mod i_account; - use i_account::{ - AccountABI, AccountABIDispatcher, AccountABIDispatcherTrait, AccountCamelABI, - AccountCamelABIDispatcher, AccountCamelABIDispatcherTrait - }; - mod i_execution_strategy; use i_execution_strategy::{ IExecutionStrategy, IExecutionStrategyDispatcher, IExecutionStrategyDispatcherTrait diff --git a/starknet/src/utils/constants.cairo b/starknet/src/utils/constants.cairo index 77cf6c91..01ecd25f 100644 --- a/starknet/src/utils/constants.cairo +++ b/starknet/src/utils/constants.cairo @@ -73,5 +73,5 @@ const U256_TYPEHASH: felt252 = 0x1094260a770342332e6a73e9256b901d484a43892531620 // ------ ERC165 Interface Ids ------ // For more information, refer to: https://github.com/starknet-io/SNIPs/blob/main/SNIPS/snip-5.md -const ERC165_ACCOUNT_INTERFACE_ID: felt252 = 0xa66bd575; // snake -const ERC165_OLD_ACCOUNT_INTERFACE_ID: felt252 = 0x3943f10f; // camel +const ERC165_ACCOUNT_INTERFACE_ID: felt252 = + 0x2ceccef7f994940b3962a6c67e0ba4fcd37df7d131417c604f91e03caecc1cd; // SNIP-6 compliant account ID, functions are snake case diff --git a/starknet/src/utils/stark_eip712.cairo b/starknet/src/utils/stark_eip712.cairo index aa54896b..0dd81ddc 100644 --- a/starknet/src/utils/stark_eip712.cairo +++ b/starknet/src/utils/stark_eip712.cairo @@ -3,15 +3,12 @@ #[starknet::contract] mod StarkEIP712 { use starknet::ContractAddress; + use openzeppelin::account::interface::{AccountABIDispatcher, AccountABIDispatcherTrait}; use sx::types::{Strategy, IndexedStrategy, Choice}; use sx::utils::StructHash; use sx::utils::constants::{ STARKNET_MESSAGE, DOMAIN_TYPEHASH, PROPOSE_TYPEHASH, VOTE_TYPEHASH, - UPDATE_PROPOSAL_TYPEHASH, ERC165_ACCOUNT_INTERFACE_ID, ERC165_OLD_ACCOUNT_INTERFACE_ID - }; - use sx::interfaces::{ - AccountABIDispatcher, AccountABIDispatcherTrait, AccountCamelABIDispatcher, - AccountCamelABIDispatcherTrait + UPDATE_PROPOSAL_TYPEHASH, ERC165_ACCOUNT_INTERFACE_ID }; #[storage] @@ -34,8 +31,7 @@ mod StarkEIP712 { metadata_uri: Span, execution_strategy: @Strategy, user_proposal_validation_params: Span, - salt: felt252, - account_type: felt252, + salt: felt252 ) { let digest: felt252 = self .get_propose_digest( @@ -47,7 +43,7 @@ mod StarkEIP712 { salt ); - InternalImpl::verify_signature(digest, signature, author, account_type); + InternalImpl::verify_signature(digest, signature, author); } /// Verifies the signature of the vote calldata. @@ -59,14 +55,13 @@ mod StarkEIP712 { proposal_id: u256, choice: Choice, user_voting_strategies: Span, - metadata_uri: Span, - account_type: felt252, + metadata_uri: Span ) { let digest: felt252 = self .get_vote_digest( target, voter, proposal_id, choice, user_voting_strategies, metadata_uri ); - InternalImpl::verify_signature(digest, signature, voter, account_type); + InternalImpl::verify_signature(digest, signature, voter); } /// Verifies the signature of the update proposal calldata. @@ -78,14 +73,13 @@ mod StarkEIP712 { proposal_id: u256, execution_strategy: @Strategy, metadata_uri: Span, - salt: felt252, - account_type: felt252, + salt: felt252 ) { let digest: felt252 = self .get_update_proposal_digest( target, author, proposal_id, execution_strategy, metadata_uri, salt ); - InternalImpl::verify_signature(digest, signature, author, account_type); + InternalImpl::verify_signature(digest, signature, author); } /// Returns the digest of the propose calldata. @@ -177,31 +171,19 @@ mod StarkEIP712 { } /// Verifies the signature of a message by calling the account contract. - fn verify_signature( - digest: felt252, - signature: Array, - account: ContractAddress, - account_type: felt252 - ) { - if account_type == 'snake' { - assert( - AccountCamelABIDispatcher { contract_address: account } - .supportsInterface(ERC165_ACCOUNT_INTERFACE_ID) == true, - 'Invalid Account' - ); - AccountCamelABIDispatcher { contract_address: account } - .isValidSignature(digest, signature); - } else if account_type == 'camel' { - assert( - AccountABIDispatcher { contract_address: account } - .supports_interface(ERC165_OLD_ACCOUNT_INTERFACE_ID) == true, - 'Invalid Account' - ); + fn verify_signature(digest: felt252, signature: Array, account: ContractAddress) { + // Only SNIP-6 compliant accounts are supported. + assert( + AccountABIDispatcher { contract_address: account } + .supports_interface(ERC165_ACCOUNT_INTERFACE_ID) == true, + 'Invalid Account' + ); + + assert( AccountABIDispatcher { contract_address: account } - .is_valid_signature(digest, signature); - } else { - panic_with_felt252('Invalid Account Type'); - } + .is_valid_signature(digest, signature) == 'VALID', + 'Invalid Signature' + ); } } } diff --git a/tests/stark-sig-auth.test.ts b/tests/stark-sig-auth.test.ts index 6f32488a..548428ea 100644 --- a/tests/stark-sig-auth.test.ts +++ b/tests/stark-sig-auth.test.ts @@ -14,16 +14,17 @@ import { dotenv.config(); const account_address = process.env.ADDRESS || ''; +const account_public_key = process.env.PUBLIC_KEY || ''; const account_pk = process.env.PK || ''; const network = process.env.STARKNET_NETWORK_URL || ''; describe('Starknet Signature Authenticator', function () { this.timeout(1000000); - // Need a starknet.js account for signing - let accountWithSigner: Account; let account: starknet.starknetAccount; - let accountType: string; + + // SNIP-6 compliant account (the defaults deployed on the devnet are not SNIP-6 compliant and therefore cannot be used for s) + let accountWithSigner: Account; let starkSigAuthenticator: starknet.StarknetContract; let vanillaVotingStrategy: starknet.StarknetContract; @@ -33,14 +34,8 @@ describe('Starknet Signature Authenticator', function () { let domain: any; before(async function () { - accountWithSigner = new Account( - new Provider({ sequencer: { baseUrl: network } }), - account_address, - account_pk, - ); account = await starknet.OpenZeppelinAccount.getAccountFromAddress(account_address, account_pk); - // change this to 'camel' if the account interface uses camel case - accountType = shortString.encodeShortString('snake'); + const accountFactory = await starknet.getContractFactory('openzeppelin_Account'); const starkSigAuthenticatorFactory = await starknet.getContractFactory( 'sx_StarkSigAuthenticator', ); @@ -54,12 +49,23 @@ describe('Starknet Signature Authenticator', function () { try { // If the contracts are already declared, this will be skipped + await account.declare(accountFactory); await account.declare(starkSigAuthenticatorFactory); await account.declare(vanillaVotingStrategyFactory); await account.declare(vanillaProposalValidationStrategyFactory); await account.declare(spaceFactory); } catch {} + const accountObj = await account.deploy(accountFactory, { + _public_key: account_public_key, + }); + + accountWithSigner = new Account( + new Provider({ sequencer: { baseUrl: network } }), + accountObj.address, + account_pk, + ); + starkSigAuthenticator = await account.deploy(starkSigAuthenticatorFactory, { name: shortString.encodeShortString('sx-sn'), version: shortString.encodeShortString('0.1.0'), @@ -109,7 +115,7 @@ describe('Starknet Signature Authenticator', function () { // PROPOSE const proposeMsg: Propose = { space: space.address, - author: account.address, + author: accountWithSigner.address, metadataUri: ['0x1', '0x2', '0x3', '0x4'], executionStrategy: { address: '0x0000000000000000000000000000000000005678', @@ -134,7 +140,6 @@ describe('Starknet Signature Authenticator', function () { const proposeCalldata = CallData.compile({ signature: [proposeSig.r, proposeSig.s], ...proposeMsg, - accountType: accountType, }); await account.invoke(starkSigAuthenticator, 'authenticate_propose', proposeCalldata, { @@ -145,7 +150,7 @@ describe('Starknet Signature Authenticator', function () { const updateProposalMsg: UpdateProposal = { space: space.address, - author: account.address, + author: accountWithSigner.address, proposalId: { low: '0x1', high: '0x0' }, executionStrategy: { address: '0x0000000000000000000000000000000000005678', @@ -165,7 +170,6 @@ describe('Starknet Signature Authenticator', function () { const updateProposalCalldata = CallData.compile({ signature: [updateProposalSig.r, updateProposalSig.s], ...updateProposalMsg, - accountType: accountType, }); await account.invoke( @@ -184,7 +188,7 @@ describe('Starknet Signature Authenticator', function () { const voteMsg: Vote = { space: space.address, - voter: account.address, + voter: accountWithSigner.address, proposalId: { low: '0x1', high: '0x0' }, choice: '0x1', userVotingStrategies: [{ index: '0x0', params: ['0x1', '0x2', '0x3', '0x4'] }], @@ -201,7 +205,6 @@ describe('Starknet Signature Authenticator', function () { const voteCalldata = CallData.compile({ signature: [voteSig.r, voteSig.s], ...voteMsg, - accountType: accountType, }); await account.invoke(starkSigAuthenticator, 'authenticate_vote', voteCalldata, { @@ -224,7 +227,7 @@ describe('Starknet Signature Authenticator', function () { // PROPOSE const proposeMsg: Propose = { space: space.address, - author: account.address, + author: accountWithSigner.address, metadataUri: ['0x1', '0x2', '0x3', '0x4'], executionStrategy: { address: '0x0000000000000000000000000000000000005678', @@ -249,7 +252,6 @@ describe('Starknet Signature Authenticator', function () { const invalidProposeCalldata = CallData.compile({ signature: [invalidProposeSig.r, invalidProposeSig.s], ...proposeMsg, - accountType: accountType, }); try { @@ -258,8 +260,7 @@ describe('Starknet Signature Authenticator', function () { }); expect.fail('Should have failed'); } catch (err: any) { - // snippet of error message thrown when signature is invalid - expect(err.message).to.contain('is invalid, with respect to the public key'); + expect(err.message).to.contain(shortString.encodeShortString('Invalid Signature')); } const proposeSig = (await accountWithSigner.signMessage({ @@ -272,7 +273,6 @@ describe('Starknet Signature Authenticator', function () { const proposeCalldata = CallData.compile({ signature: [proposeSig.r, proposeSig.s], ...proposeMsg, - accountType: accountType, }); await account.invoke(starkSigAuthenticator, 'authenticate_propose', proposeCalldata, { @@ -283,7 +283,7 @@ describe('Starknet Signature Authenticator', function () { const updateProposalMsg: UpdateProposal = { space: space.address, - author: account.address, + author: accountWithSigner.address, proposalId: { low: '0x1', high: '0x0' }, executionStrategy: { address: '0x0000000000000000000000000000000000005678', @@ -303,7 +303,6 @@ describe('Starknet Signature Authenticator', function () { const invalidUpdateProposalCalldata = CallData.compile({ signature: [invalidUpdateProposalSig.r, invalidUpdateProposalSig.s], ...updateProposalMsg, - accountType: accountType, }); try { @@ -317,8 +316,7 @@ describe('Starknet Signature Authenticator', function () { ); expect.fail('Should have failed'); } catch (err: any) { - // snippet of error message thrown when signature is invalid - expect(err.message).to.contain('is invalid, with respect to the public key'); + expect(err.message).to.contain(shortString.encodeShortString('Invalid Signature')); } const updateProposalSig = (await accountWithSigner.signMessage({ @@ -331,7 +329,6 @@ describe('Starknet Signature Authenticator', function () { const updateProposalCalldata = CallData.compile({ signature: [updateProposalSig.r, updateProposalSig.s], ...updateProposalMsg, - accountType: accountType, }); await account.invoke( @@ -350,7 +347,7 @@ describe('Starknet Signature Authenticator', function () { const voteMsg: Vote = { space: space.address, - voter: account.address, + voter: accountWithSigner.address, proposalId: { low: '0x1', high: '0x0' }, choice: '0x1', userVotingStrategies: [{ index: '0x0', params: ['0x1', '0x2', '0x3', '0x4'] }], @@ -367,7 +364,6 @@ describe('Starknet Signature Authenticator', function () { const invalidVoteCalldata = CallData.compile({ signature: [invalidVoteSig.r, invalidVoteSig.s], ...voteMsg, - accountType: accountType, }); try { @@ -376,8 +372,7 @@ describe('Starknet Signature Authenticator', function () { }); expect.fail('Should have failed'); } catch (err: any) { - // snippet of error message thrown when signature is invalid - expect(err.message).to.contain('is invalid, with respect to the public key'); + expect(err.message).to.contain(shortString.encodeShortString('Invalid Signature')); } const voteSig = (await accountWithSigner.signMessage({ @@ -390,7 +385,6 @@ describe('Starknet Signature Authenticator', function () { const voteCalldata = CallData.compile({ signature: [voteSig.r, voteSig.s], ...voteMsg, - accountType: accountType, }); await account.invoke(starkSigAuthenticator, 'authenticate_vote', voteCalldata, { @@ -406,7 +400,7 @@ describe('Starknet Signature Authenticator', function () { // PROPOSE const proposeMsg: Propose = { space: space.address, - author: account.address, + author: accountWithSigner.address, metadataUri: ['0x1', '0x2', '0x3', '0x4'], executionStrategy: { address: '0x0000000000000000000000000000000000005678', @@ -431,7 +425,6 @@ describe('Starknet Signature Authenticator', function () { const proposeCalldata = CallData.compile({ signature: [proposeSig.r, proposeSig.s], ...proposeMsg, - accountType: accountType, }); await account.invoke(starkSigAuthenticator, 'authenticate_propose', proposeCalldata, { @@ -451,7 +444,7 @@ describe('Starknet Signature Authenticator', function () { const updateProposalMsg: UpdateProposal = { space: space.address, - author: account.address, + author: accountWithSigner.address, proposalId: { low: '0x1', high: '0x0' }, executionStrategy: { address: '0x0000000000000000000000000000000000005678', @@ -471,7 +464,6 @@ describe('Starknet Signature Authenticator', function () { const updateProposalCalldata = CallData.compile({ signature: [updateProposalSig.r, updateProposalSig.s], ...updateProposalMsg, - accountType: accountType, }); await account.invoke(