Skip to content

Commit

Permalink
Feat/guardian-unmock-3: Paymaster (#870)
Browse files Browse the repository at this point in the history
* changed contracts to enforce guardian signature

* allowed redundant guardian signature

* added guardian checking in sessions

* contract: added `is_session_registered` method

* changed guardian storage from Option<felt> to felt

* Make guardian configurable by session

* `TransactionWaiter` fixed bug with not erroring when transaction reverted

* added guardian tests (failing)

* added correct session hashing to correctly sign in proxy

* adjusted tests to the new session guardian logic (1 fails)

* revert controller changes

* proxy: added guardian signature when estimating fee

* guardian tests pass

* remove `Debug` derive on `Controller`

* revert outside_execution proxy change

* session_hash name

* Support guardian as part of session

* proxy: added guardian signature when `execute_from_outside`

* changed outside execution tests to use guardian
(outside_execution + session fails)

* added guardian authorization

---------

Co-authored-by: Tarrence van As <[email protected]>
  • Loading branch information
piniom and tarrencev authored Oct 17, 2024
1 parent 8d6a32a commit ece1239
Show file tree
Hide file tree
Showing 11 changed files with 210 additions and 174 deletions.
9 changes: 5 additions & 4 deletions packages/account-wasm/src/account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use account_sdk::signers::Owner;
use serde_wasm_bindgen::to_value;
use starknet::accounts::ConnectedAccount;
use starknet::core::types::Call;
use starknet_types_core::felt::Felt;
use url::Url;
use wasm_bindgen::prelude::*;

Expand Down Expand Up @@ -123,7 +124,7 @@ impl CartridgeAccount {

let res = self
.controller
.register_session(methods, expires_at, public_key.0, max_fee.0)
.register_session(methods, expires_at, public_key.0, Felt::ZERO, max_fee.0)
.await
.map_err(JsControllerError::from)?;

Expand All @@ -141,9 +142,9 @@ impl CartridgeAccount {
.into_iter()
.map(TryFrom::try_from)
.collect::<std::result::Result<Vec<_>, _>>()?;
let call = self
.controller
.register_session_call(methods, expires_at, public_key.0)?;
let call =
self.controller
.register_session_call(methods, expires_at, public_key.0, Felt::ZERO)?;

Ok(to_value(&call.calldata)?)
}
Expand Down
2 changes: 2 additions & 0 deletions packages/account-wasm/src/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ impl CartridgeSessionAccount {
policies,
session.expires_at,
&signer.clone().into(),
Felt::ZERO,
)?;

Ok(CartridgeSessionAccount(SessionAccount::new(
Expand Down Expand Up @@ -89,6 +90,7 @@ impl CartridgeSessionAccount {
policies,
session.expires_at,
&signer.clone().into(),
Felt::ZERO,
)?;

Ok(CartridgeSessionAccount(SessionAccount::new_as_registered(
Expand Down
7 changes: 4 additions & 3 deletions packages/account_sdk/src/account/session/hash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ impl Session {
pub fn new(
policies: Vec<Policy>,
expires_at: u64,
signer: &AbigenSigner,
session_signer: &AbigenSigner,
guardian_guid: Felt,
) -> Result<Self, SignError> {
if policies.is_empty() {
return Err(SignError::NoAllowedSessionMethods);
Expand All @@ -57,8 +58,8 @@ impl Session {
policies,
authorization_root: root,
metadata: serde_json::to_string(&metadata).unwrap(),
session_key_guid: signer.clone().into(),
guardian_key_guid: Felt::ZERO,
session_key_guid: session_signer.clone().into(),
guardian_key_guid: guardian_guid,
})
}

Expand Down
22 changes: 20 additions & 2 deletions packages/account_sdk/src/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,26 @@ impl Controller {
&mut self,
methods: Vec<Policy>,
expires_at: u64,
) -> Result<SessionAccount, ControllerError> {
self.create_session_with_guardian(methods, expires_at, Felt::ZERO)
.await
}

pub async fn create_session_with_guardian(
&mut self,
methods: Vec<Policy>,
expires_at: u64,
guardian: Felt,
) -> Result<SessionAccount, ControllerError> {
let signer = SigningKey::from_random();
let session_signer = Signer::Starknet(signer.clone());

let session = Session::new(methods, expires_at, &session_signer.clone().into())?;
let session = Session::new(
methods,
expires_at,
&session_signer.clone().into(),
guardian,
)?;
let hash = session
.raw()
.get_message_hash_rev_1(self.chain_id, self.address);
Expand Down Expand Up @@ -63,12 +78,13 @@ impl Controller {
methods: Vec<Policy>,
expires_at: u64,
public_key: Felt,
guardian: Felt,
) -> Result<Call, ControllerError> {
let pubkey = VerifyingKey::from_scalar(public_key);
let signer = AbigenSigner::Starknet(StarknetSigner {
pubkey: NonZero::new(pubkey.scalar()).unwrap(),
});
let session = Session::new(methods, expires_at, &signer)?;
let session = Session::new(methods, expires_at, &signer, guardian)?;
let call = self
.contract()
.register_session_getcall(&session.raw(), &self.owner_guid());
Expand All @@ -81,6 +97,7 @@ impl Controller {
policies: Vec<Policy>,
expires_at: u64,
public_key: Felt,
guardian: Felt,
max_fee: Felt,
) -> Result<InvokeTransactionResult, ControllerError> {
let session = Session::new(
Expand All @@ -89,6 +106,7 @@ impl Controller {
&AbigenSigner::Starknet(StarknetSigner {
pubkey: NonZero::new(public_key).unwrap(),
}),
guardian,
)?;

let call = self
Expand Down
120 changes: 115 additions & 5 deletions packages/account_sdk/src/session_test.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
use cainome::cairo_serde::{ContractAddress, U256};
use starknet::{
accounts::{Account, ConnectedAccount},
core::types::{BlockId, BlockTag},
accounts::{Account, AccountError, ConnectedAccount},
core::types::{BlockId, BlockTag, StarknetError, TransactionExecutionErrorData},
macros::{felt, selector},
providers::Provider,
providers::{Provider, ProviderError},
signers::SigningKey,
};
use starknet_crypto::Felt;
Expand All @@ -15,6 +15,7 @@ use crate::{
SessionAccount,
},
artifacts::Version,
constants::GUARDIAN_SIGNER,
hash::MessageHashRev1,
signers::{Owner, Signer},
tests::{
Expand Down Expand Up @@ -160,6 +161,7 @@ async fn test_verify_execute_session_registered() {
],
u64::MAX,
&session_signer.clone().into(),
Felt::ZERO,
)
.unwrap();

Expand Down Expand Up @@ -231,7 +233,13 @@ async fn test_create_and_use_registered_session() {
let expires_at = u64::MAX;
let max_fee = Felt::from(277800000000000_u128);
let txn = controller
.register_session(policies.clone(), expires_at, public_key, max_fee)
.register_session(
policies.clone(),
expires_at,
public_key,
Felt::ZERO,
max_fee,
)
.await
.expect("Failed to register session");

Expand All @@ -240,7 +248,13 @@ async fn test_create_and_use_registered_session() {
.await
.unwrap();

let session = Session::new(policies, expires_at, &session_signer.clone().into()).unwrap();
let session = Session::new(
policies,
expires_at,
&session_signer.clone().into(),
Felt::ZERO,
)
.unwrap();

// Create a SessionAccount using new_from_registered
let session_account = SessionAccount::new_as_registered(
Expand Down Expand Up @@ -279,3 +293,99 @@ async fn test_create_and_use_registered_session() {

ensure_txn(tx, controller.provider()).await.unwrap();
}

#[tokio::test]
pub async fn test_verify_execute_with_guardian() {
let owner = Owner::Signer(Signer::new_starknet_random());
let runner = KatanaRunner::load();
let mut controller = runner
.deploy_controller("username".to_owned(), owner, Version::LATEST)
.await;

let policies = vec![
Policy::new(*FEE_TOKEN_ADDRESS, selector!("tdfs")),
Policy::new(*FEE_TOKEN_ADDRESS, selector!("transfds")),
Policy::new(*FEE_TOKEN_ADDRESS, selector!("transfer")),
];

let session_account = controller
.create_session_with_guardian(policies.clone(), u64::MAX, GUARDIAN_SIGNER.into())
.await
.unwrap();

let recipient = ContractAddress(felt!("0x18301129"));
let contract_erc20 = Erc20::new(*FEE_TOKEN_ADDRESS, &session_account);

contract_erc20
.balanceOf(&recipient)
.block_id(BlockId::Tag(BlockTag::Latest))
.call()
.await
.expect("failed to call contract");

contract_erc20
.transfer(
&recipient,
&U256 {
low: 0x10_u128,
high: 0,
},
)
.send()
.await
.unwrap();
}

#[tokio::test]
pub async fn test_verify_execute_with_invalid_guardian() {
let owner = Owner::Signer(Signer::new_starknet_random());
let runner = KatanaRunner::load();
let mut controller = runner
.deploy_controller("username".to_owned(), owner, Version::LATEST)
.await;

let policies = vec![
Policy::new(*FEE_TOKEN_ADDRESS, selector!("tdfs")),
Policy::new(*FEE_TOKEN_ADDRESS, selector!("transfds")),
Policy::new(*FEE_TOKEN_ADDRESS, selector!("transfer")),
];

let session_account = controller
.create_session_with_guardian(policies.clone(), u64::MAX, Felt::ONE)
.await
.unwrap();

let recipient = ContractAddress(felt!("0x18301129"));
let contract_erc20 = Erc20::new(*FEE_TOKEN_ADDRESS, &session_account);

contract_erc20
.balanceOf(&recipient)
.block_id(BlockId::Tag(BlockTag::Latest))
.call()
.await
.expect("failed to call contract");

let res = contract_erc20
.transfer(
&recipient,
&U256 {
low: 0x10_u128,
high: 0,
},
)
.send()
.await;

assert!(
matches!(
res,
Err(AccountError::Provider(ProviderError::StarknetError(
StarknetError::TransactionExecutionError(TransactionExecutionErrorData {
execution_error,
..
})
))) if execution_error.contains("session/invalid-guardian")
),
"Should return error"
);
}
3 changes: 3 additions & 0 deletions packages/account_sdk/src/tests/external_owners_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use starknet::{
macros::{felt, selector},
providers::Provider,
};
use starknet_crypto::Felt;

use crate::{
abigen::erc_20::Erc20,
Expand Down Expand Up @@ -45,6 +46,7 @@ async fn test_verify_external_owner() {
vec![Policy::new(*FEE_TOKEN_ADDRESS, selector!("transfer"))],
u64::MAX,
&session_signer.clone().into(),
Felt::ZERO,
)
.unwrap();

Expand Down Expand Up @@ -105,6 +107,7 @@ async fn test_verify_constructor_external_owner() {
vec![Policy::new(*FEE_TOKEN_ADDRESS, selector!("transfer"))],
u64::MAX,
&session_signer.clone().into(),
Felt::ZERO,
)
.unwrap();

Expand Down
Loading

0 comments on commit ece1239

Please sign in to comment.