From 06f214804a627f4070269ffb2e6e05573a3efb1b Mon Sep 17 00:00:00 2001 From: Jonathan LEI Date: Mon, 16 Dec 2024 10:34:53 +0800 Subject: [PATCH 1/6] refactor: encapsulate `Controller.cartridge` --- packages/keychain/src/hooks/connection.ts | 2 +- packages/keychain/src/hooks/deploy.ts | 7 ++----- packages/keychain/src/hooks/upgrade.ts | 4 ++-- packages/keychain/src/pages/session.tsx | 4 ++-- .../keychain/src/utils/connection/execute.ts | 2 +- packages/keychain/src/utils/controller.ts | 20 ++++++++++++++++++- 6 files changed, 27 insertions(+), 12 deletions(-) diff --git a/packages/keychain/src/hooks/connection.ts b/packages/keychain/src/hooks/connection.ts index 1224351bd..57e9c73a6 100644 --- a/packages/keychain/src/hooks/connection.ts +++ b/packages/keychain/src/hooks/connection.ts @@ -87,7 +87,7 @@ export function useConnectionValue() { if (controller) { posthog.identify(controller.username(), { address: controller.address, - class: controller.cartridge.classHash, + class: controller.classHash(), chainId: controller.chainId, }); } else { diff --git a/packages/keychain/src/hooks/deploy.ts b/packages/keychain/src/hooks/deploy.ts index 28e666204..d05a55d2f 100644 --- a/packages/keychain/src/hooks/deploy.ts +++ b/packages/keychain/src/hooks/deploy.ts @@ -1,11 +1,10 @@ import { useCallback, useState } from "react"; -import { num } from "starknet"; import { useConnection } from "./connection"; type TransactionHash = string; interface DeployInterface { - deploySelf: (maxFee: string) => Promise; + deploySelf: (maxFee: string) => Promise; isDeploying: boolean; } @@ -18,9 +17,7 @@ export const useDeploy = (): DeployInterface => { if (!controller) return; try { setIsDeploying(true); - const { transaction_hash } = await controller.cartridge.deploySelf( - num.toHex(maxFee), - ); + const { transaction_hash } = await controller.selfDeploy(maxFee); return transaction_hash; } catch (e) { diff --git a/packages/keychain/src/hooks/upgrade.ts b/packages/keychain/src/hooks/upgrade.ts index aeb91ad55..0ad094071 100644 --- a/packages/keychain/src/hooks/upgrade.ts +++ b/packages/keychain/src/hooks/upgrade.ts @@ -93,7 +93,7 @@ export const useUpgrade = (controller?: Controller): UpgradeInterface => { const current = CONTROLLER_VERSIONS.find( (v) => addAddressPadding(v.hash) === - addAddressPadding(controller.cartridge.classHash()), + addAddressPadding(controller.classHash()), ); setCurrent(current); setAvailable(current?.version !== LATEST_CONTROLLER.version); @@ -110,7 +110,7 @@ export const useUpgrade = (controller?: Controller): UpgradeInterface => { return []; } - return [controller.cartridge.upgrade(LATEST_CONTROLLER.hash)]; + return [controller.upgrade(LATEST_CONTROLLER.hash)]; }, [controller]); const onUpgrade = useCallback(async () => { diff --git a/packages/keychain/src/pages/session.tsx b/packages/keychain/src/pages/session.tsx index 23e37e158..630a6e194 100644 --- a/packages/keychain/src/pages/session.tsx +++ b/packages/keychain/src/pages/session.tsx @@ -107,7 +107,7 @@ export default function Session() { onCallback({ username: controller.username(), address: controller.address, - ownerGuid: controller.cartridge.ownerGuid(), + ownerGuid: controller.ownerGuid(), transactionHash: transaction_hash, expiresAt: String(SESSION_EXPIRATION), }); @@ -129,7 +129,7 @@ export default function Session() { onCallback({ username: controller.username(), address: controller.address, - ownerGuid: controller.cartridge.ownerGuid(), + ownerGuid: controller.ownerGuid(), alreadyRegistered: true, expiresAt: String(SESSION_EXPIRATION), }); diff --git a/packages/keychain/src/utils/connection/execute.ts b/packages/keychain/src/utils/connection/execute.ts index f524d7700..fd34ec065 100644 --- a/packages/keychain/src/utils/connection/execute.ts +++ b/packages/keychain/src/utils/connection/execute.ts @@ -124,7 +124,7 @@ export function execute({ } try { - let estimate = await account.cartridge.estimateInvokeFee(calls); + let estimate = await account.estimateInvokeFee(calls); const maxFee = num.toHex( num.addPercent(estimate.overall_fee, ESTIMATE_FEE_PERCENTAGE), ); diff --git a/packages/keychain/src/utils/controller.ts b/packages/keychain/src/utils/controller.ts index 77b5cf747..56e9f609e 100644 --- a/packages/keychain/src/utils/controller.ts +++ b/packages/keychain/src/utils/controller.ts @@ -26,9 +26,10 @@ import { SessionMetadata, } from "@cartridge/account-wasm/controller"; import { SessionPolicies } from "@cartridge/presets"; +import { DeployedAccountTransaction } from "@starknet-io/types-js"; export default class Controller extends Account { - cartridge: CartridgeAccount; + private cartridge: CartridgeAccount; constructor({ appId, @@ -72,6 +73,14 @@ export default class Controller extends Account { return this.cartridge.username(); } + classHash() { + return this.cartridge.classHash(); + } + + ownerGuid() { + return this.cartridge.ownerGuid(); + } + rpcUrl() { return this.cartridge.rpcUrl(); } @@ -260,6 +269,15 @@ export default class Controller extends Account { } } + async selfDeploy(maxFee: BigNumberish): Promise { + const release = await mutex.obtain(); + try { + return await this.cartridge.deploySelf(num.toHex(maxFee)); + } finally { + release(); + } + } + async delegateAccount(): Promise { const release = await mutex.obtain(); try { From 2b97d34dcaba339dbcbb8bc52605aab7be682d20 Mon Sep 17 00:00:00 2001 From: Jonathan LEI Date: Mon, 16 Dec 2024 16:13:24 +0800 Subject: [PATCH 2/6] chore: revert `b29222d` (#1137) --- .../keychain/src/utils/connection/execute.ts | 6 +- .../keychain/src/utils/connection/sign.ts | 6 +- .../keychain/src/utils/connection/sync.ts | 3 + packages/keychain/src/utils/controller.ts | 118 +++++------------- 4 files changed, 46 insertions(+), 87 deletions(-) create mode 100644 packages/keychain/src/utils/connection/sync.ts diff --git a/packages/keychain/src/utils/connection/execute.ts b/packages/keychain/src/utils/connection/execute.ts index fd34ec065..f3c96648f 100644 --- a/packages/keychain/src/utils/connection/execute.ts +++ b/packages/keychain/src/utils/connection/execute.ts @@ -11,6 +11,7 @@ import { } from "starknet"; import { ConnectionCtx, ControllerError, ExecuteCtx } from "./types"; import { ErrorCode, JsCall } from "@cartridge/account-wasm/controller"; +import { mutex } from "./sync"; export const ESTIMATE_FEE_PERCENTAGE = 10; @@ -64,6 +65,7 @@ export function execute({ }); } + const release = await mutex.obtain(); return await new Promise( async (resolve, reject) => { if (!account) { @@ -158,7 +160,9 @@ export function execute({ }); } }, - ); + ).finally(() => { + release(); + }); }; } diff --git a/packages/keychain/src/utils/connection/sign.ts b/packages/keychain/src/utils/connection/sign.ts index 8e4a52a24..cc25df3d7 100644 --- a/packages/keychain/src/utils/connection/sign.ts +++ b/packages/keychain/src/utils/connection/sign.ts @@ -5,6 +5,7 @@ import { } from "@cartridge/controller"; import { Signature, TypedData } from "starknet"; import { ConnectionCtx, SignMessageCtx } from "./types"; +import { mutex } from "./sync"; import Controller from "utils/controller"; import { parseControllerError } from "./execute"; @@ -29,6 +30,7 @@ export function signMessageFactory(setContext: (ctx: ConnectionCtx) => void) { }); } + const release = await mutex.obtain(); return await new Promise( async (resolve, reject) => { // If a session call and there is no session available @@ -62,6 +64,8 @@ export function signMessageFactory(setContext: (ctx: ConnectionCtx) => void) { }); } }, - ); + ).finally(() => { + release(); + }); }; } diff --git a/packages/keychain/src/utils/connection/sync.ts b/packages/keychain/src/utils/connection/sync.ts new file mode 100644 index 000000000..12e1a009b --- /dev/null +++ b/packages/keychain/src/utils/connection/sync.ts @@ -0,0 +1,3 @@ +import { Mutex } from "utils/mutex"; + +export const mutex = new Mutex(); diff --git a/packages/keychain/src/utils/controller.ts b/packages/keychain/src/utils/controller.ts index 56e9f609e..84cc8a373 100644 --- a/packages/keychain/src/utils/controller.ts +++ b/packages/keychain/src/utils/controller.ts @@ -13,10 +13,8 @@ import { Call, CallData, } from "starknet"; -import { Mutex } from "utils/mutex"; -import { toWasmPolicies } from "@cartridge/controller"; -const mutex = new Mutex(); +import { toWasmPolicies } from "@cartridge/controller"; import { CartridgeAccount, @@ -103,12 +101,7 @@ export default class Controller extends Account { throw new Error("Account not found"); } - const release = await mutex.obtain(); - try { - await this.cartridge.createSession(toWasmPolicies(policies), expiresAt); - } finally { - release(); - } + await this.cartridge.createSession(toWasmPolicies(policies), expiresAt); } registerSessionCalldata( @@ -133,17 +126,12 @@ export default class Controller extends Account { throw new Error("Account not found"); } - const release = await mutex.obtain(); - try { - return await this.cartridge.registerSession( - toWasmPolicies(policies), - expiresAt, - publicKey, - num.toHex(maxFee), - ); - } finally { - release(); - } + return await this.cartridge.registerSession( + toWasmPolicies(policies), + expiresAt, + publicKey, + num.toHex(maxFee), + ); } upgrade(new_class_hash: JsFelt): JsCall { @@ -151,21 +139,11 @@ export default class Controller extends Account { } async executeFromOutsideV2(calls: Call[]): Promise { - const release = await mutex.obtain(); - try { - return await this.cartridge.executeFromOutsideV2(toJsCalls(calls)); - } finally { - release(); - } + return await this.cartridge.executeFromOutsideV2(toJsCalls(calls)); } async executeFromOutsideV3(calls: Call[]): Promise { - const release = await mutex.obtain(); - try { - return await this.cartridge.executeFromOutsideV3(toJsCalls(calls)); - } finally { - release(); - } + return await this.cartridge.executeFromOutsideV3(toJsCalls(calls)); } async execute( @@ -180,15 +158,10 @@ export default class Controller extends Account { executionDetails.maxFee = num.toHex(executionDetails.maxFee); } - const release = await mutex.obtain(); - try { - return await this.cartridge.execute( - toJsCalls(calls), - executionDetails as JsInvocationsDetails, - ); - } finally { - release(); - } + return await this.cartridge.execute( + toJsCalls(calls), + executionDetails as JsInvocationsDetails, + ); } hasSession(calls: Call[]): boolean { @@ -210,27 +183,22 @@ export default class Controller extends Account { calls: Call[], _: EstimateFeeDetails = {}, ): Promise { - const release = await mutex.obtain(); - try { - const res = await this.cartridge.estimateInvokeFee(toJsCalls(calls)); - - // The reason why we set the multiplier unseemingly high is to account - // for the fact that the estimation above is done without validation (ie SKIP_VALIDATE). - // - // Setting it lower might cause the actual transaction to fail due to - // insufficient max fee. - const MULTIPLIER_PERCENTAGE = 170; // x1.7 - - // This will essentially multiply the estimated fee by 1.7 - const suggestedMaxFee = num.addPercent( - BigInt(res.overall_fee), - MULTIPLIER_PERCENTAGE, - ); + const res = await this.cartridge.estimateInvokeFee(toJsCalls(calls)); + + // The reason why we set the multiplier unseemingly high is to account + // for the fact that the estimation above is done without validation (ie SKIP_VALIDATE). + // + // Setting it lower might cause the actual transaction to fail due to + // insufficient max fee. + const MULTIPLIER_PERCENTAGE = 170; // x1.7 + + // This will essentially multiply the estimated fee by 1.7 + const suggestedMaxFee = num.addPercent( + BigInt(res.overall_fee), + MULTIPLIER_PERCENTAGE, + ); - return { suggestedMaxFee, ...res }; - } finally { - release(); - } + return { suggestedMaxFee, ...res }; } async verifyMessageHash( @@ -252,39 +220,19 @@ export default class Controller extends Account { } async signMessage(typedData: TypedData): Promise { - const release = await mutex.obtain(); - try { - return await this.cartridge.signMessage(JSON.stringify(typedData)); - } finally { - release(); - } + return this.cartridge.signMessage(JSON.stringify(typedData)); } async getNonce(_?: any): Promise { - const release = await mutex.obtain(); - try { - return await this.cartridge.getNonce(); - } finally { - release(); - } + return await this.cartridge.getNonce(); } async selfDeploy(maxFee: BigNumberish): Promise { - const release = await mutex.obtain(); - try { - return await this.cartridge.deploySelf(num.toHex(maxFee)); - } finally { - release(); - } + return await this.cartridge.deploySelf(num.toHex(maxFee)); } async delegateAccount(): Promise { - const release = await mutex.obtain(); - try { - return await this.cartridge.delegateAccount(); - } finally { - release(); - } + return this.cartridge.delegateAccount(); } revoke(_origin: string) { From bc00a3b103e46bcc0895e3e8df7f003c1781b1b9 Mon Sep 17 00:00:00 2001 From: Jonathan LEI Date: Mon, 16 Dec 2024 16:14:54 +0800 Subject: [PATCH 3/6] feat: Promise-based mutex type for wasm --- packages/account-wasm/src/lib.rs | 1 + packages/account-wasm/src/sync.rs | 70 +++++++++++++++++++++++++ packages/account-wasm/src/wasm-mutex.js | 13 +++++ 3 files changed, 84 insertions(+) create mode 100644 packages/account-wasm/src/sync.rs create mode 100644 packages/account-wasm/src/wasm-mutex.js diff --git a/packages/account-wasm/src/lib.rs b/packages/account-wasm/src/lib.rs index dff12ab66..aa3753294 100644 --- a/packages/account-wasm/src/lib.rs +++ b/packages/account-wasm/src/lib.rs @@ -5,5 +5,6 @@ pub mod account; pub mod session; mod errors; +mod sync; mod types; mod utils; diff --git a/packages/account-wasm/src/sync.rs b/packages/account-wasm/src/sync.rs new file mode 100644 index 000000000..a2f69dde2 --- /dev/null +++ b/packages/account-wasm/src/sync.rs @@ -0,0 +1,70 @@ +use std::{ + ops::{Deref, DerefMut}, + sync::{Mutex as StdMutex, MutexGuard as StdMutexGuard}, +}; + +use wasm_bindgen::prelude::*; +use wasm_bindgen_futures::js_sys::Function; + +/// A mutex implementation backed by JavaScript `Promise`s. +/// +/// This type wraps a simple JavaScript `Mutex` implementation but exposes an idiomatic Rust API. +pub struct WasmMutex { + js_lock: Mutex, + rs_lock: StdMutex, +} + +impl WasmMutex { + pub fn new(value: T) -> Self { + Self { + js_lock: Mutex::new(), + rs_lock: std::sync::Mutex::new(value), + } + } + + pub async fn lock(&self) -> WasmMutexGuard { + WasmMutexGuard { + js_release: self.js_lock.obtain().await, + // This never actually blocks as it's guarded by the JS lock. This field exists only to + // provide internal mutability for the underlying value. + rs_guard: self.rs_lock.lock().unwrap(), + } + } +} + +/// A handle to the underlying guarded value. The lock is released when the instance is dropped. +pub struct WasmMutexGuard<'a, T> { + js_release: Function, + rs_guard: StdMutexGuard<'a, T>, +} + +impl Deref for WasmMutexGuard<'_, T> { + type Target = T; + + fn deref(&self) -> &T { + std::sync::MutexGuard::deref(&self.rs_guard) + } +} + +impl DerefMut for WasmMutexGuard<'_, T> { + fn deref_mut(&mut self) -> &mut T { + std::sync::MutexGuard::deref_mut(&mut self.rs_guard) + } +} + +impl Drop for WasmMutexGuard<'_, T> { + fn drop(&mut self) { + self.js_release.call0(&JsValue::null()).unwrap(); + } +} + +#[wasm_bindgen(module = "/src/wasm-mutex.js")] +extern "C" { + type Mutex; + + #[wasm_bindgen(constructor)] + fn new() -> Mutex; + + #[wasm_bindgen(method)] + async fn obtain(this: &Mutex) -> Function; +} diff --git a/packages/account-wasm/src/wasm-mutex.js b/packages/account-wasm/src/wasm-mutex.js new file mode 100644 index 000000000..d3babff76 --- /dev/null +++ b/packages/account-wasm/src/wasm-mutex.js @@ -0,0 +1,13 @@ +function releaseStub() {} + +export class Mutex { + lastPromise = Promise.resolve(); + + async obtain() { + let release = releaseStub; + const lastPromise = this.lastPromise; + this.lastPromise = new Promise((resolve) => (release = resolve)); + await lastPromise; + return release; + } +} From 5424326aea6c93f3217b47799b54e4c640811285 Mon Sep 17 00:00:00 2001 From: Jonathan LEI Date: Mon, 16 Dec 2024 16:16:20 +0800 Subject: [PATCH 4/6] refactor: fix over-borrowing with meta type --- packages/account-wasm/src/account.rs | 141 ++++++++++++++++------ packages/keychain/src/utils/controller.ts | 30 +++-- 2 files changed, 126 insertions(+), 45 deletions(-) diff --git a/packages/account-wasm/src/account.rs b/packages/account-wasm/src/account.rs index 7c95830ef..84be4e9c4 100644 --- a/packages/account-wasm/src/account.rs +++ b/packages/account-wasm/src/account.rs @@ -40,6 +40,7 @@ impl CartridgeAccount { /// - `username`: Username associated with the account. /// - `signer`: A Signer struct containing the signer type and associated data. /// + #[allow(clippy::new_ret_no_self)] pub fn new( app_id: String, class_hash: JsFelt, @@ -48,7 +49,7 @@ impl CartridgeAccount { address: JsFelt, username: String, signer: Signer, - ) -> Result { + ) -> Result { set_panic_hook(); let rpc_url = Url::parse(&rpc_url)?; @@ -65,47 +66,22 @@ impl CartridgeAccount { chain_id.0, ); - Ok(CartridgeAccount { controller }) + Ok(CartridgeAccountWithMeta::new(controller)) } #[wasm_bindgen(js_name = fromStorage)] - pub fn from_storage(app_id: String) -> Result> { + pub fn from_storage(app_id: String) -> Result> { set_panic_hook(); let controller = Controller::from_storage(app_id).map_err(|e| JsError::new(&e.to_string()))?; match controller { - Some(c) => Ok(Some(CartridgeAccount { controller: c })), + Some(c) => Ok(Some(CartridgeAccountWithMeta::new(c))), None => Ok(None), } } - #[wasm_bindgen(js_name = username)] - pub fn username(&self) -> String { - self.controller.username.clone() - } - - #[wasm_bindgen(js_name = address)] - pub fn address(&self) -> String { - self.controller.address.to_hex_string() - } - - #[wasm_bindgen(js_name = classHash)] - pub fn class_hash(&self) -> String { - self.controller.class_hash.to_hex_string() - } - - #[wasm_bindgen(js_name = rpcUrl)] - pub fn rpc_url(&self) -> String { - self.controller.rpc_url.to_string() - } - - #[wasm_bindgen(js_name = chainId)] - pub fn chain_id(&self) -> String { - self.controller.chain_id.to_string() - } - #[wasm_bindgen(js_name = disconnect)] pub fn disconnect(&mut self) -> std::result::Result<(), JsControllerError> { self.controller @@ -113,11 +89,6 @@ impl CartridgeAccount { .map_err(JsControllerError::from) } - #[wasm_bindgen(js_name = ownerGuid)] - pub fn owner_guid(&self) -> JsFelt { - JsFelt(self.controller.owner_guid()) - } - #[wasm_bindgen(js_name = registerSession)] pub async fn register_session( &mut self, @@ -352,3 +323,105 @@ impl CartridgeAccount { Ok(JsFelt(res)) } } + +/// A type for accessing fixed attributes of `CartridgeAccount`. +/// +/// This type exists as concurrent mutable and immutable calls to `CartridgeAccount` are guarded +/// with `WasmMutex`, which only operates under an `async` context. If these getters were directly +/// implemented under `CartridgeAccount`: +/// +/// - calls to them would unnecessarily have to be `async` as well; +/// - there would be excessive locking. +/// +/// This type is supposed to only ever be borrowed immutably. So no concurrent access control would +/// be needed. +#[wasm_bindgen] +#[derive(Clone)] +pub struct CartridgeAccountMeta { + username: String, + address: String, + class_hash: String, + rpc_url: String, + chain_id: String, + owner_guid: JsFelt, +} + +impl CartridgeAccountMeta { + fn new(controller: &Controller) -> Self { + Self { + username: controller.username.clone(), + address: controller.address.to_hex_string(), + class_hash: controller.class_hash.to_hex_string(), + rpc_url: controller.rpc_url.to_string(), + chain_id: controller.chain_id.to_string(), + owner_guid: JsFelt(controller.owner_guid()), + } + } +} + +#[wasm_bindgen] +impl CartridgeAccountMeta { + #[wasm_bindgen(js_name = username)] + pub fn username(&self) -> String { + self.username.clone() + } + + #[wasm_bindgen(js_name = address)] + pub fn address(&self) -> String { + self.address.clone() + } + + #[wasm_bindgen(js_name = classHash)] + pub fn class_hash(&self) -> String { + self.class_hash.clone() + } + + #[wasm_bindgen(js_name = rpcUrl)] + pub fn rpc_url(&self) -> String { + self.rpc_url.clone() + } + + #[wasm_bindgen(js_name = chainId)] + pub fn chain_id(&self) -> String { + self.chain_id.clone() + } + + #[wasm_bindgen(js_name = ownerGuid)] + pub fn owner_guid(&self) -> JsFelt { + self.owner_guid.clone() + } +} + +/// A type used as the return type for constructing `CartridgeAccount` to provide an extra, +/// separately borrowable `meta` field for synchronously accessing fixed fields. +/// +/// This type exists instead of simply having `CartridgeAccount::new()` return a tuple as tuples +/// don't implement `IntoWasmAbi` which is needed for crossing JS-WASM boundary. +#[wasm_bindgen] +pub struct CartridgeAccountWithMeta { + account: CartridgeAccount, + meta: CartridgeAccountMeta, +} + +impl CartridgeAccountWithMeta { + fn new(controller: Controller) -> Self { + let meta = CartridgeAccountMeta::new(&controller); + Self { + account: CartridgeAccount { controller }, + meta, + } + } +} + +#[wasm_bindgen] +impl CartridgeAccountWithMeta { + #[wasm_bindgen(js_name = meta)] + pub fn meta(&self) -> CartridgeAccountMeta { + self.meta.clone() + } + + #[wasm_bindgen(js_name = intoAccount)] + pub fn into_account(self) -> CartridgeAccount { + self.account + } +} diff --git a/packages/keychain/src/utils/controller.ts b/packages/keychain/src/utils/controller.ts index 84cc8a373..681b258eb 100644 --- a/packages/keychain/src/utils/controller.ts +++ b/packages/keychain/src/utils/controller.ts @@ -18,6 +18,7 @@ import { toWasmPolicies } from "@cartridge/controller"; import { CartridgeAccount, + CartridgeAccountMeta, JsCall, JsFelt, JsInvocationsDetails, @@ -28,6 +29,7 @@ import { DeployedAccountTransaction } from "@starknet-io/types-js"; export default class Controller extends Account { private cartridge: CartridgeAccount; + private cartridgeMeta: CartridgeAccountMeta; constructor({ appId, @@ -50,7 +52,7 @@ export default class Controller extends Account { }) { super({ nodeUrl: rpcUrl }, address, ""); - this.cartridge = CartridgeAccount.new( + const accountWithMeta = CartridgeAccount.new( appId, classHash, rpcUrl, @@ -65,26 +67,29 @@ export default class Controller extends Account { }, }, ); + + this.cartridgeMeta = accountWithMeta.meta(); + this.cartridge = accountWithMeta.intoAccount(); } username() { - return this.cartridge.username(); + return this.cartridgeMeta.username(); } classHash() { - return this.cartridge.classHash(); + return this.cartridgeMeta.classHash(); } ownerGuid() { - return this.cartridge.ownerGuid(); + return this.cartridgeMeta.ownerGuid(); } rpcUrl() { - return this.cartridge.rpcUrl(); + return this.cartridgeMeta.rpcUrl(); } chainId() { - return this.cartridge.chainId(); + return this.cartridgeMeta.chainId(); } disconnect() { @@ -241,19 +246,22 @@ export default class Controller extends Account { } static fromStore(appId: string) { - let cartridge = CartridgeAccount.fromStorage(appId); - if (!cartridge) { + const cartridgeWithMeta = CartridgeAccount.fromStorage(appId); + if (!cartridgeWithMeta) { return; } + const meta = cartridgeWithMeta.meta(); + const controller = new Account( - { nodeUrl: cartridge.rpcUrl() }, - cartridge.address(), + { nodeUrl: meta.rpcUrl() }, + meta.address(), "", ) as Controller; Object.setPrototypeOf(controller, Controller.prototype); - controller.cartridge = cartridge; + controller.cartridge = cartridgeWithMeta.intoAccount(); + controller.cartridgeMeta = meta; return controller; } } From 7b70092d27089cc81d1db2a1c9e83ff5275d6ac8 Mon Sep 17 00:00:00 2001 From: Jonathan LEI Date: Mon, 16 Dec 2024 16:16:52 +0800 Subject: [PATCH 5/6] feat: `CartridgeAccount` internal mutability --- packages/account-wasm/src/account.rs | 96 ++++++++++++++----- .../src/components/ConfirmTransaction.tsx | 18 +++- .../components/connect/RegisterSession.tsx | 42 ++++---- packages/keychain/src/hooks/connection.ts | 11 ++- packages/keychain/src/hooks/upgrade.ts | 14 ++- packages/keychain/src/pages/index.tsx | 16 +++- packages/keychain/src/pages/session.tsx | 24 ++--- .../keychain/src/utils/connection/execute.ts | 2 +- .../keychain/src/utils/connection/index.ts | 10 +- .../keychain/src/utils/connection/probe.ts | 5 +- .../keychain/src/utils/connection/sign.ts | 2 +- packages/keychain/src/utils/controller.ts | 28 +++--- 12 files changed, 179 insertions(+), 89 deletions(-) diff --git a/packages/account-wasm/src/account.rs b/packages/account-wasm/src/account.rs index 84be4e9c4..d4c45cdaf 100644 --- a/packages/account-wasm/src/account.rs +++ b/packages/account-wasm/src/account.rs @@ -1,3 +1,5 @@ +use std::borrow::BorrowMut; + use account_sdk::account::session::policy::Policy as SdkPolicy; use account_sdk::controller::Controller; use account_sdk::errors::ControllerError; @@ -13,6 +15,7 @@ use url::Url; use wasm_bindgen::prelude::*; use crate::errors::JsControllerError; +use crate::sync::WasmMutex; use crate::types::call::JsCall; use crate::types::invocation::JsInvocationsDetails; use crate::types::policy::Policy; @@ -25,7 +28,7 @@ type Result = std::result::Result; #[wasm_bindgen] pub struct CartridgeAccount { - controller: Controller, + controller: WasmMutex, } #[wasm_bindgen] @@ -83,15 +86,17 @@ impl CartridgeAccount { } #[wasm_bindgen(js_name = disconnect)] - pub fn disconnect(&mut self) -> std::result::Result<(), JsControllerError> { + pub async fn disconnect(&self) -> std::result::Result<(), JsControllerError> { self.controller + .lock() + .await .disconnect() .map_err(JsControllerError::from) } #[wasm_bindgen(js_name = registerSession)] pub async fn register_session( - &mut self, + &self, policies: Vec, expires_at: u64, public_key: JsFelt, @@ -104,6 +109,8 @@ impl CartridgeAccount { let res = self .controller + .lock() + .await .register_session(methods, expires_at, public_key.0, Felt::ZERO, max_fee.0) .await .map_err(JsControllerError::from)?; @@ -112,8 +119,8 @@ impl CartridgeAccount { } #[wasm_bindgen(js_name = registerSessionCalldata)] - pub fn register_session_calldata( - &mut self, + pub async fn register_session_calldata( + &self, policies: Vec, expires_at: u64, public_key: JsFelt, @@ -122,19 +129,22 @@ impl CartridgeAccount { .into_iter() .map(TryFrom::try_from) .collect::, _>>()?; - let call = - self.controller - .register_session_call(methods, expires_at, public_key.0, Felt::ZERO)?; + let call = self.controller.lock().await.register_session_call( + methods, + expires_at, + public_key.0, + Felt::ZERO, + )?; Ok(to_value(&call.calldata)?) } #[wasm_bindgen(js_name = upgrade)] - pub fn upgrade( + pub async fn upgrade( &self, new_class_hash: JsFelt, ) -> std::result::Result { - let call = self.controller.upgrade(new_class_hash.0); + let call = self.controller.lock().await.upgrade(new_class_hash.0); Ok(JsCall { contract_address: call.to, entrypoint: "upgrade".to_string(), @@ -143,7 +153,7 @@ impl CartridgeAccount { } #[wasm_bindgen(js_name = createSession)] - pub async fn create_session(&mut self, policies: Vec, expires_at: u64) -> Result<()> { + pub async fn create_session(&self, policies: Vec, expires_at: u64) -> Result<()> { set_panic_hook(); let methods = policies @@ -151,7 +161,11 @@ impl CartridgeAccount { .map(TryFrom::try_from) .collect::, _>>()?; - self.controller.create_session(methods, expires_at).await?; + self.controller + .lock() + .await + .create_session(methods, expires_at) + .await?; Ok(()) } @@ -168,13 +182,18 @@ impl CartridgeAccount { .map(TryFrom::try_from) .collect::, _>>()?; - let fee_estimate = self.controller.estimate_invoke_fee(calls).await?; + let fee_estimate = self + .controller + .lock() + .await + .estimate_invoke_fee(calls) + .await?; Ok(to_value(&fee_estimate)?) } #[wasm_bindgen(js_name = execute)] pub async fn execute( - &mut self, + &self, calls: Vec, details: JsInvocationsDetails, ) -> std::result::Result { @@ -185,14 +204,19 @@ impl CartridgeAccount { .map(TryFrom::try_from) .collect::, _>>()?; - let result = Controller::execute(&mut self.controller, calls, details.max_fee).await?; + let result = Controller::execute( + self.controller.lock().await.borrow_mut(), + calls, + details.max_fee, + ) + .await?; Ok(to_value(&result)?) } #[wasm_bindgen(js_name = executeFromOutsideV2)] pub async fn execute_from_outside_v2( - &mut self, + &self, calls: Vec, ) -> std::result::Result { set_panic_hook(); @@ -202,13 +226,18 @@ impl CartridgeAccount { .map(TryInto::try_into) .collect::>()?; - let response = self.controller.execute_from_outside_v2(calls).await?; + let response = self + .controller + .lock() + .await + .execute_from_outside_v2(calls) + .await?; Ok(to_value(&response)?) } #[wasm_bindgen(js_name = executeFromOutsideV3)] pub async fn execute_from_outside_v3( - &mut self, + &self, calls: Vec, ) -> std::result::Result { set_panic_hook(); @@ -218,12 +247,17 @@ impl CartridgeAccount { .map(TryInto::try_into) .collect::>()?; - let response = self.controller.execute_from_outside_v3(calls).await?; + let response = self + .controller + .lock() + .await + .execute_from_outside_v3(calls) + .await?; Ok(to_value(&response)?) } #[wasm_bindgen(js_name = hasSession)] - pub fn has_session(&self, calls: Vec) -> Result { + pub async fn has_session(&self, calls: Vec) -> Result { let calls: Vec = calls .into_iter() .map(TryFrom::try_from) @@ -231,12 +265,14 @@ impl CartridgeAccount { Ok(self .controller + .lock() + .await .session_account(&SdkPolicy::from_calls(&calls)) .is_some()) } #[wasm_bindgen(js_name = hasSessionForMessage)] - pub fn has_session_for_message(&self, typed_data: String) -> Result { + pub async fn has_session_for_message(&self, typed_data: String) -> Result { let typed_data: TypedData = serde_json::from_str(&typed_data)?; let domain_hash = typed_data.domain.encode(&typed_data.types)?; let type_hash = @@ -245,12 +281,14 @@ impl CartridgeAccount { Ok(self .controller + .lock() + .await .session_account(&[SdkPolicy::new_typed_data(scope_hash)]) .is_some()) } #[wasm_bindgen(js_name = session)] - pub fn session_metadata( + pub async fn session_metadata( &self, policies: Vec, public_key: Option, @@ -262,6 +300,8 @@ impl CartridgeAccount { Ok(self .controller + .lock() + .await .session_metadata(&policies, public_key.map(|f| f.0)) .map(|(_, metadata)| SessionMetadata::from(metadata))) } @@ -277,6 +317,8 @@ impl CartridgeAccount { let signature = self .controller + .lock() + .await .sign_message(serde_json::from_str(&typed_data)?) .await .map_err(|e| JsControllerError::from(ControllerError::SignError(e)))?; @@ -288,6 +330,8 @@ impl CartridgeAccount { pub async fn get_nonce(&self) -> std::result::Result { let nonce = self .controller + .lock() + .await .get_nonce() .await .map_err(|e| JsControllerError::from(ControllerError::ProviderError(e)))?; @@ -301,6 +345,8 @@ impl CartridgeAccount { let res = self .controller + .lock() + .await .deploy() .max_fee(max_fee.0) .send() @@ -316,6 +362,8 @@ impl CartridgeAccount { let res = self .controller + .lock() + .await .delegate_account() .await .map_err(JsControllerError::from)?; @@ -407,7 +455,9 @@ impl CartridgeAccountWithMeta { fn new(controller: Controller) -> Self { let meta = CartridgeAccountMeta::new(&controller); Self { - account: CartridgeAccount { controller }, + account: CartridgeAccount { + controller: WasmMutex::new(controller), + }, meta, } } diff --git a/packages/keychain/src/components/ConfirmTransaction.tsx b/packages/keychain/src/components/ConfirmTransaction.tsx index b60f52d5f..9141fdec9 100644 --- a/packages/keychain/src/components/ConfirmTransaction.tsx +++ b/packages/keychain/src/components/ConfirmTransaction.tsx @@ -1,4 +1,4 @@ -import { useMemo, useState } from "react"; +import { useEffect, useMemo, useState } from "react"; import { ResponseCodes, SessionPolicies, toArray } from "@cartridge/controller"; import { Content, FOOTER_MIN_HEIGHT } from "components/layout"; import { TransactionDuoIcon } from "@cartridge/ui"; @@ -12,6 +12,7 @@ import { CreateSession } from "./connect"; export function ConfirmTransaction() { const { controller, context, origin, policies, setContext } = useConnection(); const [policiesUpdated, setIsPoliciesUpdated] = useState(false); + const [updateSession, setUpdateSession] = useState(false); const ctx = context as ExecuteCtx; const account = controller; @@ -46,8 +47,11 @@ export function ConfirmTransaction() { [ctx.transactions], ); - const updateSession = useMemo(() => { - if (policiesUpdated) return false; + useEffect(() => { + if (policiesUpdated) { + setUpdateSession(false); + return; + } const entries = Object.entries(callPolicies.contracts || {}); const txnsApproved = entries.every(([target, policy]) => { @@ -60,7 +64,13 @@ export function ConfirmTransaction() { // If calls are approved by dapp specified policies but not stored session // then prompt user to update session. This also accounts for expired sessions. - return txnsApproved && !account?.session(callPolicies); + if (txnsApproved && account) { + account.session(callPolicies).then((hasSession) => { + setUpdateSession(!hasSession); + }); + } else { + setUpdateSession(false); + } }, [callPolicies, policiesUpdated, policies, account]); if (updateSession && policies) { diff --git a/packages/keychain/src/components/connect/RegisterSession.tsx b/packages/keychain/src/components/connect/RegisterSession.tsx index f5db1af5b..dac9c7180 100644 --- a/packages/keychain/src/components/connect/RegisterSession.tsx +++ b/packages/keychain/src/components/connect/RegisterSession.tsx @@ -1,5 +1,5 @@ import { Content } from "components/layout"; -import { useCallback, useMemo, useState } from "react"; +import { useCallback, useEffect, useState } from "react"; import { useConnection } from "hooks/connection"; import { SessionConsent } from "components/connect"; import { ExecutionContainer } from "components/ExecutionContainer"; @@ -23,23 +23,31 @@ export function RegisterSession({ }) { const { controller, theme } = useConnection(); const [expiresAt] = useState(SESSION_EXPIRATION); + const [transactions, setTransactions] = useState< + | { + contractAddress: string; + entrypoint: string; + calldata: string[]; + }[] + | undefined + >(undefined); - const transactions = useMemo(() => { - if (!publicKey || !controller) return; - - const calldata = controller.registerSessionCalldata( - expiresAt, - policies, - publicKey, - ); - - return [ - { - contractAddress: controller.address, - entrypoint: "register_session", - calldata, - }, - ]; + useEffect(() => { + if (!publicKey || !controller) { + setTransactions(undefined); + } else { + controller + .registerSessionCalldata(expiresAt, policies, publicKey) + .then((calldata) => { + setTransactions([ + { + contractAddress: controller.address, + entrypoint: "register_session", + calldata, + }, + ]); + }); + } }, [controller, expiresAt, policies, publicKey]); const onRegisterSession = useCallback( diff --git a/packages/keychain/src/hooks/connection.ts b/packages/keychain/src/hooks/connection.ts index 57e9c73a6..e9e043cf7 100644 --- a/packages/keychain/src/hooks/connection.ts +++ b/packages/keychain/src/hooks/connection.ts @@ -224,12 +224,13 @@ export function useConnectionValue() { }, [rpcUrl, controller]); const logout = useCallback(() => { - window.controller?.disconnect(); - setController(undefined); + window.controller?.disconnect().then(() => { + setController(undefined); - context?.resolve?.({ - code: ResponseCodes.NOT_CONNECTED, - message: "User logged out", + context?.resolve?.({ + code: ResponseCodes.NOT_CONNECTED, + message: "User logged out", + }); }); }, [context, setController]); diff --git a/packages/keychain/src/hooks/upgrade.ts b/packages/keychain/src/hooks/upgrade.ts index 0ad094071..5efa4ce72 100644 --- a/packages/keychain/src/hooks/upgrade.ts +++ b/packages/keychain/src/hooks/upgrade.ts @@ -1,4 +1,5 @@ -import { useCallback, useEffect, useMemo, useState } from "react"; +import { JsCall } from "@cartridge/account-wasm"; +import { useCallback, useEffect, useState } from "react"; import { addAddressPadding, Call } from "starknet"; import { ControllerError } from "utils/connection"; import Controller from "utils/controller"; @@ -71,6 +72,7 @@ export const useUpgrade = (controller?: Controller): UpgradeInterface => { const [isSynced, setIsSynced] = useState(false); const [isUpgrading, setIsUpgrading] = useState(false); const [current, setCurrent] = useState(); + const [calls, setCalls] = useState([]); useEffect(() => { if (!controller) { @@ -105,12 +107,14 @@ export const useUpgrade = (controller?: Controller): UpgradeInterface => { .finally(() => setIsSynced(true)); }, [controller]); - const calls = useMemo(() => { + useEffect(() => { if (!controller || !LATEST_CONTROLLER) { - return []; + setCalls([]); + } else { + controller.upgrade(LATEST_CONTROLLER.hash).then((call) => { + setCalls([call]); + }); } - - return [controller.upgrade(LATEST_CONTROLLER.hash)]; }, [controller]); const onUpgrade = useCallback(async () => { diff --git a/packages/keychain/src/pages/index.tsx b/packages/keychain/src/pages/index.tsx index c2106386d..56893430d 100644 --- a/packages/keychain/src/pages/index.tsx +++ b/packages/keychain/src/pages/index.tsx @@ -10,11 +10,13 @@ import { ErrorPage } from "components/ErrorBoundary"; import { Settings } from "components/Settings"; import { Upgrade } from "components/connect/Upgrade"; import { PurchaseCredits } from "components/Funding/PurchaseCredits"; -import { useEffect } from "react"; +import { useEffect, useState } from "react"; import { usePostHog } from "posthog-js/react"; function Home() { const { context, controller, error, policies, upgrade } = useConnection(); + const [hasSessionForPolicies, setHasSessionForPolicies] = + useState(false); const posthog = usePostHog(); useEffect(() => { @@ -33,6 +35,16 @@ function Home() { } }, [context?.origin, posthog]); + useEffect(() => { + if (controller && policies) { + controller.session(policies).then((session) => { + setHasSessionForPolicies(!!session); + }); + } else { + setHasSessionForPolicies(false); + } + }, [controller, policies]); + if (window.self === window.top || !context?.origin) { return <>; } @@ -73,7 +85,7 @@ function Home() { return <>; } - if (controller.session(policies)) { + if (hasSessionForPolicies) { context.resolve({ code: ResponseCodes.SUCCESS, address: controller.address, diff --git a/packages/keychain/src/pages/session.tsx b/packages/keychain/src/pages/session.tsx index 630a6e194..fb0d94d97 100644 --- a/packages/keychain/src/pages/session.tsx +++ b/packages/keychain/src/pages/session.tsx @@ -125,19 +125,21 @@ export default function Session() { // If the requested policies has no mismatch with existing policies and public key already // registered then return the exising session - if (controller.session(policies, queries.public_key)) { - onCallback({ - username: controller.username(), - address: controller.address, - ownerGuid: controller.ownerGuid(), - alreadyRegistered: true, - expiresAt: String(SESSION_EXPIRATION), - }); + controller.session(policies, queries.public_key).then((session) => { + if (session) { + onCallback({ + username: controller.username(), + address: controller.address, + ownerGuid: controller.ownerGuid(), + alreadyRegistered: true, + expiresAt: String(SESSION_EXPIRATION), + }); - return; - } + return; + } - setIsLoading(false); + setIsLoading(false); + }); }, [controller, origin, policies, queries.public_key, onCallback]); if (!controller) { diff --git a/packages/keychain/src/utils/connection/execute.ts b/packages/keychain/src/utils/connection/execute.ts index f3c96648f..5609d0c46 100644 --- a/packages/keychain/src/utils/connection/execute.ts +++ b/packages/keychain/src/utils/connection/execute.ts @@ -76,7 +76,7 @@ export function execute({ // If a session call and there is no session available // fallback to manual apporval flow - if (!account.hasSession(calls)) { + if (!(await account.hasSession(calls))) { setContext({ type: "execute", origin, diff --git a/packages/keychain/src/utils/connection/index.ts b/packages/keychain/src/utils/connection/index.ts index 0427b12f5..6b7b99046 100644 --- a/packages/keychain/src/utils/connection/index.ts +++ b/packages/keychain/src/utils/connection/index.ts @@ -47,12 +47,14 @@ export function connectToController({ reset: () => () => setContext(undefined), fetchControllers: fetchControllers, disconnect: () => () => { - window.controller?.disconnect(); - setController(undefined); + window.controller?.disconnect().then(() => { + setController(undefined); + }); }, logout: () => () => { - window.controller?.disconnect(); - setController(undefined); + window.controller?.disconnect().then(() => { + setController(undefined); + }); }, username: () => () => window.controller?.username(), delegateAccount: () => () => window.controller?.delegateAccount(), diff --git a/packages/keychain/src/utils/connection/probe.ts b/packages/keychain/src/utils/connection/probe.ts index a792ff94a..812497dc9 100644 --- a/packages/keychain/src/utils/connection/probe.ts +++ b/packages/keychain/src/utils/connection/probe.ts @@ -18,8 +18,9 @@ export function probeFactory({ } if (rpcUrl !== controller.rpcUrl()) { - controller.disconnect(); - setController(undefined); + controller.disconnect().then(() => { + setController(undefined); + }); return Promise.reject({ code: ResponseCodes.NOT_CONNECTED, }); diff --git a/packages/keychain/src/utils/connection/sign.ts b/packages/keychain/src/utils/connection/sign.ts index cc25df3d7..2faa3f49c 100644 --- a/packages/keychain/src/utils/connection/sign.ts +++ b/packages/keychain/src/utils/connection/sign.ts @@ -35,7 +35,7 @@ export function signMessageFactory(setContext: (ctx: ConnectionCtx) => void) { async (resolve, reject) => { // If a session call and there is no session available // fallback to manual apporval flow - if (!controller.hasSessionForMessage(typedData)) { + if (!(await controller.hasSessionForMessage(typedData))) { setContext({ type: "sign-message", origin, diff --git a/packages/keychain/src/utils/controller.ts b/packages/keychain/src/utils/controller.ts index 681b258eb..f643abe97 100644 --- a/packages/keychain/src/utils/controller.ts +++ b/packages/keychain/src/utils/controller.ts @@ -92,8 +92,8 @@ export default class Controller extends Account { return this.cartridgeMeta.chainId(); } - disconnect() { - this.cartridge.disconnect(); + async disconnect() { + await this.cartridge.disconnect(); delete window.controller; } @@ -109,12 +109,12 @@ export default class Controller extends Account { await this.cartridge.createSession(toWasmPolicies(policies), expiresAt); } - registerSessionCalldata( + async registerSessionCalldata( expiresAt: bigint, policies: SessionPolicies, publicKey: string, - ): Array { - return this.cartridge.registerSessionCalldata( + ): Promise> { + return await this.cartridge.registerSessionCalldata( toWasmPolicies(policies), expiresAt, publicKey, @@ -139,8 +139,8 @@ export default class Controller extends Account { ); } - upgrade(new_class_hash: JsFelt): JsCall { - return this.cartridge.upgrade(new_class_hash); + async upgrade(new_class_hash: JsFelt): Promise { + return await this.cartridge.upgrade(new_class_hash); } async executeFromOutsideV2(calls: Call[]): Promise { @@ -169,19 +169,19 @@ export default class Controller extends Account { ); } - hasSession(calls: Call[]): boolean { - return this.cartridge.hasSession(toJsCalls(calls)); + async hasSession(calls: Call[]): Promise { + return await this.cartridge.hasSession(toJsCalls(calls)); } - hasSessionForMessage(typedData: TypedData): boolean { - return this.cartridge.hasSessionForMessage(JSON.stringify(typedData)); + async hasSessionForMessage(typedData: TypedData): Promise { + return await this.cartridge.hasSessionForMessage(JSON.stringify(typedData)); } - session( + async session( policies: SessionPolicies, public_key?: string, - ): SessionMetadata | undefined { - return this.cartridge.session(toWasmPolicies(policies), public_key); + ): Promise { + return await this.cartridge.session(toWasmPolicies(policies), public_key); } async estimateInvokeFee( From 6e47dcb70e8140ee1653ad7ac5e2453f1cdf2c60 Mon Sep 17 00:00:00 2001 From: Jonathan LEI Date: Tue, 17 Dec 2024 22:17:53 +0800 Subject: [PATCH 6/6] feat: loading screen for policy matching --- packages/keychain/src/pages/index.tsx | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/packages/keychain/src/pages/index.tsx b/packages/keychain/src/pages/index.tsx index 56893430d..f8b56fe30 100644 --- a/packages/keychain/src/pages/index.tsx +++ b/packages/keychain/src/pages/index.tsx @@ -12,11 +12,13 @@ import { Upgrade } from "components/connect/Upgrade"; import { PurchaseCredits } from "components/Funding/PurchaseCredits"; import { useEffect, useState } from "react"; import { usePostHog } from "posthog-js/react"; +import { PageLoading } from "components/Loading"; function Home() { const { context, controller, error, policies, upgrade } = useConnection(); - const [hasSessionForPolicies, setHasSessionForPolicies] = - useState(false); + const [hasSessionForPolicies, setHasSessionForPolicies] = useState< + boolean | undefined + >(undefined); const posthog = usePostHog(); useEffect(() => { @@ -41,7 +43,7 @@ function Home() { setHasSessionForPolicies(!!session); }); } else { - setHasSessionForPolicies(false); + setHasSessionForPolicies(undefined); } }, [controller, policies]); @@ -85,7 +87,10 @@ function Home() { return <>; } - if (hasSessionForPolicies) { + if (hasSessionForPolicies === undefined) { + // This is likely never observable in a real application but just in case. + return ; + } else if (hasSessionForPolicies) { context.resolve({ code: ResponseCodes.SUCCESS, address: controller.address,