feat(pxe): disallow registering invalid addresses as senders

yarn-project/pxe/src/logs/log_service.ts

@@ -174,7 +174,6 @@ export class LogService {
 
         if (!secret) {
           // Note that all senders originate from either the SenderAddressBookStore or the KeyStore.
-          // TODO(F-512): make sure we actually prevent registering invalid senders.
           throw new Error(
             `Failed to compute a tagging secret for sender ${sender} - this implies this is an invalid address, which should not happen as they have been previously registered in PXE.`,
           );

yarn-project/pxe/src/pxe.test.ts

@@ -97,6 +97,12 @@ describe('PXE', () => {
     expect(accounts).toContainEqual(completeAddress);
   });
 
+  it('refuses to register an invalid address as a sender', async () => {
+    // x = 3 is not a valid x-coordinate on the Grumpkin curve (y^2 = x^3 - 17 = 10 has no square root in Fr)
+    const invalidAddress = new AztecAddress(new Fr(3));
+    await expect(pxe.registerSender(invalidAddress)).rejects.toThrow(/not valid/);
+  });
+
   it('does not throw when registering the same account twice (just ignores the second attempt)', async () => {
     const randomSecretKey = Fr.random();
     const randomPartialAddress = Fr.random();

yarn-project/pxe/src/pxe.ts

@@ -560,6 +560,12 @@ export class PXE {
    * TODO: It's strange that we return the address here and I (benesjan) think we should drop the return value.
    */
   public async registerSender(sender: AztecAddress): Promise<AztecAddress> {
+    if (!(await sender.isValid())) {
+      throw new Error(
+        `Address ${sender} is not valid: it does not correspond to a point on the Grumpkin curve. Cannot register it as a sender.`,
+      );
+    }
+
     const accounts = await this.keyStore.getAccounts();
     if (accounts.includes(sender)) {
       this.log.info(`Sender:\n "${sender.toString()}"\n already registered.`);