Merge pull request #1464 from julio4/feat/safe-sender

Feat: safe sender

CHANGELOG.md

@@ -25,6 +25,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
   - `Provable.runAndCheck()`, `Provable.constraintSystem()` and `{SmartContract,ZkProgram}.digest()` are also async now
   - These changes were made to add internal support for async circuits
   - `Provable.runAndCheckSync()` added and immediately deprecated for a smoother upgrade path for tests
+- Remove `this.sender` which unintuitively did not prove that its value was the actual sender of the transaction https://github.com/o1-labs/o1js/pull/1464 [@julio4](https://github.com/julio4)
+  Replaced by more explicit APIs:
+  - `this.sender.getUnconstrained()` which has the old behavior of `this.sender`, and returns an unconstrained value (which means that the prover can set it to any value they want)
+  - `this.sender.getAndRequireSignature()` which requires a signature from the sender's public key and therefore proves that whoever created the transaction really owns the sender account
 - `Reducer.reduce()` requires the maximum number of actions per method as an explicit (optional) argument https://github.com/o1-labs/o1js/pull/1450
   - The default value is 1 and should work for most existing contracts
 - `new UInt64()` and `UInt64.from()` no longer unsafely accept a field element as input. https://github.com/o1-labs/o1js/pull/1438 [@julio4](https://github.com/julio4)  

src/examples/zkapps/dex/dex-with-actions.ts

@@ -79,7 +79,11 @@ class Dex extends TokenContract {
   }
 
   @method createAccount() {
-    this.internal.mint({ address: this.sender, amount: UInt64.from(0) });
+    this.internal.mint({
+      // unconstrained because we don't care which account is created
+      address: this.sender.getUnconstrained(),
+      amount: UInt64.from(0),
+    });
   }
 
   /**
@@ -93,7 +97,8 @@ class Dex extends TokenContract {
    * instead, the input X and Y amounts determine the initial ratio.
    */
   @method supplyLiquidityBase(dx: UInt64, dy: UInt64): UInt64 {
-    let user = this.sender;
+    // unconstrained because `transfer()` requires sender signature anyway
+    let user = this.sender.getUnconstrained();
     let tokenX = new TrivialCoin(this.tokenX);
     let tokenY = new TrivialCoin(this.tokenY);
 
@@ -164,14 +169,15 @@ class Dex extends TokenContract {
    * contracts pay you tokens when reducing the action.
    */
   @method redeemInitialize(dl: UInt64) {
-    this.reducer.dispatch(new RedeemAction({ address: this.sender, dl }));
-    this.internal.burn({ address: this.sender, amount: dl });
+    let sender = this.sender.getUnconstrained(); // unconstrained because `burn()` requires sender signature anyway
+    this.reducer.dispatch(new RedeemAction({ address: sender, dl }));
+    this.internal.burn({ address: sender, amount: dl });
     // TODO: preconditioning on the state here ruins concurrent interactions,
     // there should be another `finalize` DEX method which reduces actions & updates state
     this.totalSupply.set(this.totalSupply.getAndRequireEquals().sub(dl));
 
     // emit event
-    this.typedEvents.emit('redeem-liquidity', { address: this.sender, dl });
+    this.typedEvents.emit('redeem-liquidity', { address: sender, dl });
   }
 
   /**
@@ -194,10 +200,11 @@ class Dex extends TokenContract {
    * the called methods which requires proof authorization.
    */
   swapX(dx: UInt64): UInt64 {
+    let user = this.sender.getUnconstrained(); // unconstrained because `swap()` requires sender signature anyway
     let tokenY = new TrivialCoin(this.tokenY);
     let dexY = new DexTokenHolder(this.address, tokenY.deriveTokenId());
-    let dy = dexY.swap(this.sender, dx, this.tokenX);
-    tokenY.transfer(dexY.self, this.sender, dy);
+    let dy = dexY.swap(user, dx, this.tokenX);
+    tokenY.transfer(dexY.self, user, dy);
     return dy;
   }
 
@@ -212,10 +219,11 @@ class Dex extends TokenContract {
    * the called methods which requires proof authorization.
    */
   swapY(dy: UInt64): UInt64 {
+    let user = this.sender.getUnconstrained(); // unconstrained because `swap()` requires sender signature anyway
     let tokenX = new TrivialCoin(this.tokenX);
     let dexX = new DexTokenHolder(this.address, tokenX.deriveTokenId());
-    let dx = dexX.swap(this.sender, dy, this.tokenY);
-    tokenX.transfer(dexX.self, this.sender, dx);
+    let dx = dexX.swap(user, dy, this.tokenY);
+    tokenX.transfer(dexX.self, user, dx);
     return dx;
   }
 }
@@ -312,7 +320,7 @@ class DexTokenHolder extends SmartContract {
     this.balance.subInPlace(dy);
 
     // emit event
-    this.typedEvents.emit('swap', { address: this.sender, dx });
+    this.typedEvents.emit('swap', { address: user, dx });
 
     return dy;
   }

src/examples/zkapps/dex/dex.ts

@@ -55,7 +55,7 @@ function createDex({
      * instead, the input X and Y amounts determine the initial ratio.
      */
     @method supplyLiquidityBase(dx: UInt64, dy: UInt64): UInt64 {
-      let user = this.sender;
+      let user = this.sender.getUnconstrained(); // unconstrained because transfer() requires the signature anyway
       let tokenX = new TokenContract(this.tokenX);
       let tokenY = new TokenContract(this.tokenY);
 
@@ -148,11 +148,12 @@ function createDex({
      */
     redeemLiquidity(dl: UInt64) {
       // call the token X holder inside a token X-approved callback
+      let sender = this.sender.getUnconstrained(); // unconstrained because redeemLiquidity() requires the signature anyway
       let tokenX = new TokenContract(this.tokenX);
       let dexX = new DexTokenHolder(this.address, tokenX.deriveTokenId());
-      let dxdy = dexX.redeemLiquidity(this.sender, dl, this.tokenY);
+      let dxdy = dexX.redeemLiquidity(sender, dl, this.tokenY);
       let dx = dxdy[0];
-      tokenX.transfer(dexX.self, this.sender, dx);
+      tokenX.transfer(dexX.self, sender, dx);
       return dxdy;
     }
 
@@ -164,10 +165,11 @@ function createDex({
      * The transaction needs to be signed by the user's private key.
      */
     @method swapX(dx: UInt64): UInt64 {
+      let sender = this.sender.getUnconstrained(); // unconstrained because swap() requires the signature anyway
       let tokenY = new TokenContract(this.tokenY);
       let dexY = new DexTokenHolder(this.address, tokenY.deriveTokenId());
-      let dy = dexY.swap(this.sender, dx, this.tokenX);
-      tokenY.transfer(dexY.self, this.sender, dy);
+      let dy = dexY.swap(sender, dx, this.tokenX);
+      tokenY.transfer(dexY.self, sender, dy);
       return dy;
     }
 
@@ -179,10 +181,11 @@ function createDex({
      * The transaction needs to be signed by the user's private key.
      */
     @method swapY(dy: UInt64): UInt64 {
+      let sender = this.sender.getUnconstrained(); // unconstrained because swap() requires the signature anyway
       let tokenX = new TokenContract(this.tokenX);
       let dexX = new DexTokenHolder(this.address, tokenX.deriveTokenId());
-      let dx = dexX.swap(this.sender, dy, this.tokenY);
-      tokenX.transfer(dexX.self, this.sender, dx);
+      let dx = dexX.swap(sender, dy, this.tokenY);
+      tokenX.transfer(dexX.self, sender, dx);
       return dx;
     }
 
@@ -215,13 +218,14 @@ function createDex({
     }
 
     @method swapX(dx: UInt64): UInt64 {
+      let sender = this.sender.getUnconstrained(); // unconstrained because swap() requires the signature anyway
       let tokenY = new TokenContract(this.tokenY);
       let dexY = new ModifiedDexTokenHolder(
         this.address,
         tokenY.deriveTokenId()
       );
-      let dy = dexY.swap(this.sender, dx, this.tokenX);
-      tokenY.transfer(dexY.self, this.sender, dy);
+      let dy = dexY.swap(sender, dx, this.tokenX);
+      tokenY.transfer(dexY.self, sender, dy);
       return dy;
     }
   }

src/examples/zkapps/simple-zkapp-payment.ts

@@ -22,11 +22,14 @@ class SendMINAExample extends SmartContract {
   }
 
   @method withdraw(amount: UInt64) {
-    this.send({ to: this.sender, amount });
+    // unconstrained because we don't care where the user wants to withdraw to
+    let to = this.sender.getUnconstrained();
+    this.send({ to, amount });
   }
 
   @method deposit(amount: UInt64) {
-    let senderUpdate = AccountUpdate.createSigned(this.sender);
+    let sender = this.sender.getUnconstrained(); // unconstrained because we're already requiring a signature in the next line
+    let senderUpdate = AccountUpdate.createSigned(sender);
     senderUpdate.send({ to: this, amount });
   }
 }

src/lib/mina/account-update-layout.unit-test.ts

@@ -7,12 +7,14 @@ import { SmartContract, method } from '../zkapp.js';
 
 class NestedCall extends SmartContract {
   @method deposit() {
-    let payerUpdate = AccountUpdate.createSigned(this.sender);
+    let sender = this.sender.getUnconstrained();
+    let payerUpdate = AccountUpdate.createSigned(sender);
     payerUpdate.send({ to: this.address, amount: UInt64.one });
   }
 
   @method depositUsingTree() {
-    let payerUpdate = AccountUpdate.createSigned(this.sender);
+    let sender = this.sender.getUnconstrained();
+    let payerUpdate = AccountUpdate.createSigned(sender);
     let receiverUpdate = AccountUpdate.create(this.address);
     payerUpdate.send({ to: receiverUpdate, amount: UInt64.one });
 

src/lib/zkapp.ts

@@ -821,31 +821,40 @@ super.init();
 
   #_senderState: { sender: PublicKey; transactionId: number };
 
-  /**
-   * The public key of the current transaction's sender account.
-   *
-   * Throws an error if not inside a transaction, or the sender wasn't passed in.
-   *
-   * **Warning**: The fact that this public key equals the current sender is not part of the proof.
-   * A malicious prover could use any other public key without affecting the validity of the proof.
-   */
-  get sender(): PublicKey {
-    // TODO this logic now has some overlap with this.self, we should combine them somehow
-    // (but with care since the logic in this.self is a bit more complicated)
-    if (!Mina.currentTransaction.has()) {
-      throw Error(
-        `this.sender is not available outside a transaction. Make sure you only use it within \`Mina.transaction\` blocks or smart contract methods.`
-      );
-    }
-    let transactionId = Mina.currentTransaction.id();
-    if (this.#_senderState?.transactionId === transactionId) {
-      return this.#_senderState.sender;
-    } else {
-      let sender = Provable.witness(PublicKey, () => Mina.sender());
-      this.#_senderState = { transactionId, sender };
+  sender = {
+    self: this as SmartContract,
+    /**
+     * The public key of the current transaction's sender account.
+     *
+     * Throws an error if not inside a transaction, or the sender wasn't passed in.
+     *
+     * **Warning**: The fact that this public key equals the current sender is not part of the proof.
+     * A malicious prover could use any other public key without affecting the validity of the proof.
+     */
+    getUnconstrained(): PublicKey {
+      // TODO this logic now has some overlap with this.self, we should combine them somehow
+      // (but with care since the logic in this.self is a bit more complicated)
+      if (!Mina.currentTransaction.has()) {
+        throw Error(
+          `this.sender is not available outside a transaction. Make sure you only use it within \`Mina.transaction\` blocks or smart contract methods.`
+        );
+      }
+      let transactionId = Mina.currentTransaction.id();
+      if (this.self.#_senderState?.transactionId === transactionId) {
+        return this.self.#_senderState.sender;
+      } else {
+        let sender = Provable.witness(PublicKey, () => Mina.sender());
+        this.self.#_senderState = { transactionId, sender };
+        return sender;
+      }
+    },
+
+    getAndRequireSignature(): PublicKey {
+      let sender = this.getUnconstrained();
+      AccountUpdate.createSigned(sender);
       return sender;
-    }
-  }
+    },
+  };
 
   /**
    * Current account of the {@link SmartContract}.

tests/integration/simple-zkapp.js

@@ -61,7 +61,8 @@ class NotSoSimpleZkapp extends SmartContract {
   }
 
   deposit(amount) {
-    let senderUpdate = AccountUpdate.createSigned(this.sender);
+    let sender = this.sender.getUnconstrained(); // unconstrained because we're already requiring a signature in the next line
+    let senderUpdate = AccountUpdate.createSigned(sender);
     senderUpdate.send({ to: this, amount });
   }
 }