[protocol 3.6] Allow initializing owner with account update txs (#1990)

Author: Brechtpd

packages/loopring_v3.js/src/request_processors/account_update_processor.ts

@@ -20,10 +20,15 @@ interface AccountUpdate {
  * Processes account update requests.
  */
 export class AccountUpdateProcessor {
-  public static process(state: ExchangeState, block: BlockContext, txData: Bitstream) {
+  public static process(
+    state: ExchangeState,
+    block: BlockContext,
+    txData: Bitstream
+  ) {
     const update = AccountUpdateProcessor.extractData(txData);
 
     const account = state.getAccount(update.accountID);
+    account.owner = update.owner;
     account.publicKeyX = update.publicKeyX;
     account.publicKeyY = update.publicKeyY;
     account.nonce++;
@@ -50,7 +55,10 @@ export class AccountUpdateProcessor {
     offset += 4;
     update.feeTokenID = data.extractUint16(offset);
     offset += 2;
-    update.fee = fromFloat(data.extractUint16(offset), Constants.Float16Encoding);
+    update.fee = fromFloat(
+      data.extractUint16(offset),
+      Constants.Float16Encoding
+    );
     offset += 2;
     const publicKey = data.extractData(offset, 32);
     offset += 32;

packages/loopring_v3/circuit/Circuits/AccountUpdateCircuit.h

@@ -36,6 +36,7 @@ class AccountUpdateCircuit : public BaseTransactionCircuit
     Poseidon_gadget_T<9, 1, 6, 53, 8, 1> hash;
 
     // Validate
+    OwnerValidGadget ownerValid;
     RequireLtGadget requireValidUntil;
     RequireLeqGadget requireValidFee;
 
@@ -67,7 +68,7 @@ class AccountUpdateCircuit : public BaseTransactionCircuit
         : BaseTransactionCircuit(pb, state, prefix),
 
           // Inputs
-          owner(pb, state.accountA.account.owner, NUM_BITS_ADDRESS, FMT(prefix, ".owner")),
+          owner(pb, NUM_BITS_ADDRESS, FMT(prefix, ".owner")),
           accountID(pb, NUM_BITS_ACCOUNT, FMT(prefix, ".accountID")),
           validUntil(pb, NUM_BITS_TIMESTAMP, FMT(prefix, ".validUntil")),
           nonce(pb, state.accountA.account.nonce, NUM_BITS_NONCE, FMT(prefix, ".nonce")),
@@ -93,6 +94,7 @@ class AccountUpdateCircuit : public BaseTransactionCircuit
             FMT(this->annotation_prefix, ".hash")),
 
           // Validate
+          ownerValid(pb, state.constants, state.accountA.account.owner, owner.packed, FMT(prefix, ".ownerValid")),
           requireValidUntil(
             pb,
             state.timestamp,
@@ -145,6 +147,7 @@ class AccountUpdateCircuit : public BaseTransactionCircuit
     {
         // Update the account data
         setArrayOutput(TXV_ACCOUNT_A_ADDRESS, accountID.bits);
+        setOutput(TXV_ACCOUNT_A_OWNER, owner.packed);
         setOutput(TXV_ACCOUNT_A_PUBKEY_X, publicKeyX);
         setOutput(TXV_ACCOUNT_A_PUBKEY_Y, publicKeyY);
         setOutput(TXV_ACCOUNT_A_NONCE, nonce_after.result());
@@ -168,7 +171,7 @@ class AccountUpdateCircuit : public BaseTransactionCircuit
     void generate_r1cs_witness(const AccountUpdateTx &update)
     {
         // Inputs
-        owner.generate_r1cs_witness();
+        owner.generate_r1cs_witness(pb, update.owner);
         accountID.generate_r1cs_witness(pb, update.accountID);
         validUntil.generate_r1cs_witness(pb, update.validUntil);
         nonce.generate_r1cs_witness();
@@ -183,6 +186,7 @@ class AccountUpdateCircuit : public BaseTransactionCircuit
         hash.generate_r1cs_witness();
 
         // Validate
+        ownerValid.generate_r1cs_witness();
         requireValidUntil.generate_r1cs_witness();
         requireValidFee.generate_r1cs_witness();
 
@@ -224,6 +228,7 @@ class AccountUpdateCircuit : public BaseTransactionCircuit
         hash.generate_r1cs_constraints();
 
         // Validate
+        ownerValid.generate_r1cs_constraints();
         requireValidUntil.generate_r1cs_constraints();
         requireValidFee.generate_r1cs_constraints();
 

packages/loopring_v3/circuit/Utils/Data.h

@@ -381,6 +381,7 @@ static void from_json(const json &j, Withdrawal &withdrawal)
 class AccountUpdateTx
 {
   public:
+    ethsnarks::FieldT owner;
     ethsnarks::FieldT accountID;
     ethsnarks::FieldT publicKeyX;
     ethsnarks::FieldT publicKeyY;
@@ -393,6 +394,7 @@ class AccountUpdateTx
 
 static void from_json(const json &j, AccountUpdateTx &update)
 {
+    update.owner = ethsnarks::FieldT(j.at("owner").get<std::string>().c_str());
     update.accountID = ethsnarks::FieldT(j.at("accountID"));
     update.publicKeyX = ethsnarks::FieldT(j["publicKeyX"].get<std::string>().c_str());
     update.publicKeyY = ethsnarks::FieldT(j["publicKeyY"].get<std::string>().c_str());
@@ -557,6 +559,8 @@ static void from_json(const json &j, UniversalTransaction &transaction)
     // Patch some of the dummy tx's so they are valid against the current state
     // Deposit
     transaction.deposit.owner = transaction.witness.accountUpdate_A.before.owner;
+    // AccountUpdate
+    transaction.accountUpdate.owner = transaction.witness.accountUpdate_A.before.owner;
     // Transfer
     transaction.transfer.to = transaction.witness.accountUpdate_B.before.owner;
     transaction.transfer.payerTo = transaction.witness.accountUpdate_B.before.owner;

packages/loopring_v3/operator/state.py

@@ -684,6 +684,7 @@ def executeTransaction(self, context, txInput):
             newState.TXV_ACCOUNT_A_ADDRESS = txInput.accountID
             accountA = self.getAccount(newState.TXV_ACCOUNT_A_ADDRESS)
 
+            newState.TXV_ACCOUNT_A_OWNER = txInput.owner
             newState.TXV_ACCOUNT_A_PUBKEY_X = txInput.publicKeyX
             newState.TXV_ACCOUNT_A_PUBKEY_Y = txInput.publicKeyY
             newState.TXV_ACCOUNT_A_NONCE = 1

packages/loopring_v3/test/simulator.ts

@@ -57,11 +57,7 @@ export class Balance {
     this.storage = {};
   }
 
-  public init(
-    balance: BN,
-    weightAMM: BN,
-    storage: { [key: number]: Storage }
-  ) {
+  public init(balance: BN, weightAMM: BN, storage: { [key: number]: Storage }) {
     this.balance = new BN(balance.toString(10));
     this.weightAMM = new BN(weightAMM.toString(10));
     this.storage = storage;
@@ -178,9 +174,7 @@ export class Simulator {
           const jBalance = jAccount._balancesLeafs[balanceKey];
 
           const storage: { [key: number]: Storage } = {};
-          const storageKeys: string[] = Object.keys(
-            jBalance._storageLeafs
-          );
+          const storageKeys: string[] = Object.keys(jBalance._storageLeafs);
           for (const storageKey of storageKeys) {
             const jStorage = jBalance._storageLeafs[storageKey];
             storage[Number(storageKey)] = {
@@ -322,7 +316,11 @@ export class Simulator {
       "pubKeyY does not match"
     );
     assert.equal(accountA.nonce, accountB.nonce, "nonce does not match");
-    assert.equal(accountA.feeBipsAMM, accountB.feeBipsAMM, "feeBipsAMM does not match");
+    assert.equal(
+      accountA.feeBipsAMM,
+      accountB.feeBipsAMM,
+      "feeBipsAMM does not match"
+    );
   }
 
   public static executeBlock(
@@ -504,6 +502,7 @@ export class Simulator {
     update: AccountUpdate
   ) {
     const account = state.getAccount(update.accountID);
+    account.owner = update.owner;
     account.publicKeyX = update.publicKeyX;
     account.publicKeyY = update.publicKeyY;
     account.nonce++;
@@ -554,7 +553,9 @@ export class Simulator {
     from.getBalance(transfer.feeTokenID).balance.isub(transfer.fee);
 
     // Nonce
-    const storage = from.getBalance(transfer.tokenID).getStorage(transfer.storageID);
+    const storage = from
+      .getBalance(transfer.tokenID)
+      .getStorage(transfer.storageID);
     storage.data = new BN(1);
     storage.storageID = transfer.storageID;
 
@@ -581,19 +582,17 @@ export class Simulator {
       amount = new BN(0);
     }
     account.getBalance(withdrawal.tokenID).balance.isub(amount);
-    account
-      .getBalance(withdrawal.feeTokenID)
-      .balance.isub(withdrawal.fee);
+    account.getBalance(withdrawal.feeTokenID).balance.isub(withdrawal.fee);
 
     const operator = state.getAccount(block.operatorAccountID);
-    operator
-      .getBalance(withdrawal.feeTokenID)
-      .balance.iadd(withdrawal.fee);
+    operator.getBalance(withdrawal.feeTokenID).balance.iadd(withdrawal.fee);
 
     if (withdrawal.type === 0 || withdrawal.type === 1) {
       // Nonce
       const storageSlot = withdrawal.storageID % Constants.NUM_STORAGE_SLOTS;
-      const storage = account.getBalance(withdrawal.tokenID).getStorage(storageSlot);
+      const storage = account
+        .getBalance(withdrawal.tokenID)
+        .getStorage(storageSlot);
       storage.storageID = withdrawal.storageID;
       storage.data = new BN(1);
     }
@@ -907,7 +906,8 @@ export class Simulator {
         .isub(s.feeA);
 
       const tradeHistoryA = accountA.getBalance(tokenA).getStorage(storageIdA);
-      tradeHistoryA.data = storageIdA > tradeHistoryA.storageID ? new BN(0) : tradeHistoryA.data;
+      tradeHistoryA.data =
+        storageIdA > tradeHistoryA.storageID ? new BN(0) : tradeHistoryA.data;
       tradeHistoryA.data.iadd(fillAmountBorSA ? s.fillBA : s.fillSA);
       tradeHistoryA.storageID = storageIdA;
     }
@@ -921,7 +921,8 @@ export class Simulator {
         .isub(s.feeB);
 
       const tradeHistoryB = accountB.getBalance(tokenB).getStorage(storageIdB);
-      tradeHistoryB.data = storageIdB > tradeHistoryB.storageID ? new BN(0) : tradeHistoryB.data;
+      tradeHistoryB.data =
+        storageIdB > tradeHistoryB.storageID ? new BN(0) : tradeHistoryB.data;
       tradeHistoryB.data.iadd(fillAmountBorSB ? s.fillBB : s.fillSB);
       tradeHistoryB.storageID = storageIdB;
     }
@@ -1074,8 +1075,10 @@ export class Simulator {
       .getBalance(order.tokenIdS)
       .getStorage(order.storageID);
     // Trade history trimming
-    const leafStorageID = tradeHistory.storageID === 0 ? storageSlot : tradeHistory.storageID;
-    const filled = leafStorageID === order.storageID ? tradeHistory.data : new BN(0);
+    const leafStorageID =
+      tradeHistory.storageID === 0 ? storageSlot : tradeHistory.storageID;
+    const filled =
+      leafStorageID === order.storageID ? tradeHistory.data : new BN(0);
     return filled;
   }
 

packages/loopring_v3/test/testExchangeAccounts.ts

@@ -40,6 +40,31 @@ contract("Exchange", (accounts: string[]) => {
   describe("Accounts", function() {
     this.timeout(0);
 
+    it("Should be able to create an account", async () => {
+      await createExchange();
+
+      const token = ctx.getTokenAddress("LRC");
+      const fee = new BN(0);
+
+      // Create the account
+      let newKeyPair = ctx.getKeyPairEDDSA();
+      await ctx.requestAccountUpdate(ownerA, token, fee, newKeyPair, {
+        authMethod: AuthMethod.ECDSA
+      });
+
+      // Submit
+      await ctx.submitTransactions();
+      await ctx.submitPendingBlocks();
+
+      // Try to create the account with EdDSA
+      await ctx.requestAccountUpdate(ownerB, token, fee, newKeyPair, {
+        authMethod: AuthMethod.EDDSA
+      });
+
+      // Submit
+      await expectThrow(ctx.submitTransactions(), "invalid block");
+    });
+
     it("Should be able to update an account", async () => {
       await createExchange();
 

packages/loopring_v3/test/testExchangeUtil.ts

@@ -1402,7 +1402,20 @@ export class ExchangeTestUtil {
     }
     const feeTokenID = this.tokenAddressToIDMap.get(feeToken);
 
-    const account = this.findAccount(owner);
+    let isNewAccount = false;
+    let account = this.findAccount(owner);
+    if (account === undefined) {
+      account = {
+        accountID: this.accounts[this.exchangeId].length,
+        owner: owner,
+        publicKeyX: "0",
+        publicKeyY: "0",
+        secretKey: "0",
+        nonce: 0
+      };
+      this.accounts[this.exchangeId].push(account);
+      isNewAccount = true;
+    }
 
     const accountUpdate: AccountUpdate = {
       txType: "AccountUpdate",
@@ -1422,7 +1435,12 @@ export class ExchangeTestUtil {
 
     // Sign the public key update
     if (authMethod === AuthMethod.EDDSA) {
-      accountUpdate.signature = AccountUpdateUtils.sign(account, accountUpdate);
+      // New accounts should not be able to set keys with EDDSA.
+      // Try to sign with the keys we're setting (which shouldn't work).
+      accountUpdate.signature = AccountUpdateUtils.sign(
+        isNewAccount ? keyPair : account,
+        accountUpdate
+      );
     } else if (authMethod === AuthMethod.ECDSA) {
       const hash = AccountUpdateUtils.getHash(
         accountUpdate,