Skip to content

Commit

Permalink
refactor: fix over-borrowing with meta type
Browse files Browse the repository at this point in the history
  • Loading branch information
xJonathanLEI committed Dec 17, 2024
1 parent bc00a3b commit 5424326
Show file tree
Hide file tree
Showing 2 changed files with 126 additions and 45 deletions.
141 changes: 107 additions & 34 deletions packages/account-wasm/src/account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -48,7 +49,7 @@ impl CartridgeAccount {
address: JsFelt,
username: String,
signer: Signer,
) -> Result<CartridgeAccount> {
) -> Result<CartridgeAccountWithMeta> {
set_panic_hook();

let rpc_url = Url::parse(&rpc_url)?;
Expand All @@ -65,59 +66,29 @@ 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<Option<CartridgeAccount>> {
pub fn from_storage(app_id: String) -> Result<Option<CartridgeAccountWithMeta>> {
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
.disconnect()
.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,
Expand Down Expand Up @@ -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
}
}
30 changes: 19 additions & 11 deletions packages/keychain/src/utils/controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { toWasmPolicies } from "@cartridge/controller";

import {
CartridgeAccount,
CartridgeAccountMeta,
JsCall,
JsFelt,
JsInvocationsDetails,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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() {
Expand Down Expand Up @@ -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;
}
}
Expand Down

0 comments on commit 5424326

Please sign in to comment.