fix: H-02 Votes Can Be Blocked

starknet/src/authenticators/eth_tx.cairo

@@ -69,12 +69,13 @@ mod EthTxAuthenticator {
     use starknet::{ContractAddress, EthAddress, Felt252TryIntoEthAddress, EthAddressIntoFelt252,};
     use sx::interfaces::{ISpaceDispatcher, ISpaceDispatcherTrait};
     use sx::types::{UserAddress, Strategy, IndexedStrategy, Choice};
+    use sx::utils::LegacyHashFelt252EthAddress;
     use sx::utils::constants::{PROPOSE_SELECTOR, VOTE_SELECTOR, UPDATE_PROPOSAL_SELECTOR};
 
     #[storage]
     struct Storage {
         _starknet_commit_address: EthAddress,
-        _commits: LegacyMap::<felt252, EthAddress>
+        _commits: LegacyMap::<(felt252, EthAddress), bool>
     }
 
     #[external(v0)]
@@ -176,19 +177,17 @@ mod EthTxAuthenticator {
         assert(
             from_address == self._starknet_commit_address.read().into(), 'Invalid commit address'
         );
-        // Prevents hash being overwritten by a different sender.
-        assert(self._commits.read(hash).into() == 0, 'Commit already exists');
-        self._commits.write(hash, sender_address.try_into().unwrap());
+        let sender_address = sender_address.try_into().unwrap();
+        assert(self._commits.read((hash, sender_address)) == false, 'Commit already exists');
+        self._commits.write((hash, sender_address), true);
     }
 
     #[generate_trait]
     impl InternalImpl of InternalTrait {
         fn consume_commit(ref self: ContractState, hash: felt252, sender_address: EthAddress) {
-            let committer_address = self._commits.read(hash);
-            assert(committer_address.is_non_zero(), 'Commit not found');
-            assert(committer_address == sender_address, 'Invalid sender address');
+            assert(self._commits.read((hash, sender_address)), 'Commit not found');
             // Delete the commit to prevent replay attacks.
-            self._commits.write(hash, Zeroable::zero());
+            self._commits.write((hash, sender_address), false);
         }
     }
 }

starknet/src/lib.cairo

@@ -118,8 +118,8 @@ mod utils {
 
     mod legacy_hash;
     use legacy_hash::{
-        LegacyHashEthAddress, LegacyHashUsedSalts, LegacyHashChoice, LegacyHashUserAddress,
-        LegacyHashVotePower, LegacyHashVoteRegistry, LegacyHashSpanFelt252
+        LegacyHashEthAddress, LegacyHashFelt252EthAddress, LegacyHashUsedSalts, LegacyHashChoice,
+        LegacyHashUserAddress, LegacyHashVotePower, LegacyHashVoteRegistry, LegacyHashSpanFelt252
     };
 
     mod math;

starknet/src/utils/legacy_hash.cairo

@@ -15,6 +15,14 @@ impl LegacyHashEthAddress of LegacyHash<EthAddress> {
     }
 }
 
+impl LegacyHashFelt252EthAddress of LegacyHash<(felt252, EthAddress)> {
+    fn hash(state: felt252, value: (felt252, EthAddress)) -> felt252 {
+        let (_felt252, _eth_address) = value;
+        let state = LegacyHash::hash(state, _felt252);
+        LegacyHash::hash(state, _eth_address)
+    }
+}
+
 impl LegacyHashSpanFelt252 of LegacyHash<Span<felt252>> {
     fn hash(state: felt252, mut value: Span<felt252>) -> felt252 {
         let len = value.len();

tests/eth-tx-auth.test.ts

@@ -473,10 +473,68 @@ describe('Ethereum Transaction Authenticator', function () {
       );
       expect.fail('Should have failed');
     } catch (err: any) {
-      expect(err.message).to.contain(shortString.encodeShortString('Invalid sender address'));
+      expect(err.message).to.contain(shortString.encodeShortString('Commit not found'));
     }
   }, 1000000);
 
+  it('should not revert if the same commit was made twice', async () => {
+    await starknet.devnet.restart();
+    await starknet.devnet.load('./dump.pkl');
+    await starknet.devnet.increaseTime(10);
+    await starknet.devnet.loadL1MessagingContract(eth_network, mockStarknetMessaging.address);
+
+    const proposal = {
+      author: signer.address,
+      metadataUri: ['0x1', '0x2', '0x3', '0x4'],
+      executionStrategy: {
+        address: '0x0000000000000000000000000000000000005678',
+        params: ['0x0'],
+      },
+      userProposalValidationParams: [
+        '0xffffffffffffffffffffffffffffffffffffffffff',
+        '0x1234',
+        '0x5678',
+        '0x9abc',
+      ],
+    };
+
+    const proposeCommitPreImage = CallData.compile({
+      target: space.address,
+      selector: selector.getSelectorFromName('propose'),
+      ...proposal,
+    });
+
+    // Commit hash of payload to the Starknet Commit L1 contract
+    const commit = `0x${poseidonHashMany(proposeCommitPreImage.map((v) => BigInt(v))).toString(
+      16,
+    )}`;
+
+    // Committing payload from a different address to the author
+    await starknetCommit
+      .connect(invalidSigner)
+      .commit(ethTxAuthenticator.address, commit, { value: 18485000000000 });
+
+    expect((await starknet.devnet.flush()).consumed_messages.from_l1).to.have.a.lengthOf(1);
+
+    // Committing the same payload from the author
+    await starknetCommit.commit(ethTxAuthenticator.address, commit, { value: 18485000000000 });
+
+    expect((await starknet.devnet.flush()).consumed_messages.from_l1).to.have.a.lengthOf(1);
+
+    await account.invoke(
+      ethTxAuthenticator,
+      'authenticate_propose',
+      CallData.compile({
+        target: space.address,
+        author: proposal.author,
+        metadataURI: proposal.metadataUri,
+        executionStrategy: proposal.executionStrategy,
+        userProposalValidationParams: proposal.userProposalValidationParams,
+      }),
+      { rawInput: true },
+    );
+  }, 1000000);
+
   it('a commit cannot be consumed twice', async () => {
     await starknet.devnet.restart();
     await starknet.devnet.load('./dump.pkl');