Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Whitelist selectors on execute_action #43

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions gas-report.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,16 @@ Summary:
│ Transfer STRK (FeeToken: WEI) │ '828.000.000.320' │ 0.0033 │ 828000000000 │ 23 │ 21 │ 2 │ 1 │ 'steps' │ 4 │ 320 │ 'BLOB' │
│ Gifting WEI (FeeToken: WEI) │ '1.548.000.000.288' │ 0.0061 │ 1548000000000 │ 43 │ 37 │ 5 │ 2 │ 'steps' │ 3 │ 288 │ 'BLOB' │
│ Claiming WEI (FeeToken: WEI) │ '1.188.000.000.192' │ 0.0047 │ 1188000000000 │ 33 │ 30 │ 2 │ 2 │ 'steps' │ 3 │ 192 │ 'BLOB' │
│ Claiming external WEI (FeeToken: WEI) │ '1.512.000.000.256' │ 0.00615120000000004239 │ 2 │ 2 │ 'steps' │ 4 │ 256 │ 'BLOB' │
│ Claiming external WEI (FeeToken: WEI) │ '1.656.000.000.256' │ 0.006616560000000004643 │ 2 │ 2 │ 'steps' │ 4 │ 256 │ 'BLOB' │
│ Gifting WEI (FeeToken: FRI) │ '2.196.000.000.480' │ 0.0087 │ 2196000000000 │ 61 │ 52 │ 7 │ 3 │ 'steps' │ 5 │ 480 │ 'BLOB' │
│ Claiming WEI (FeeToken: FRI) │ '1.188.000.000.320' │ 0 │ 1188000000000 │ 33 │ 30 │ 2 │ 2 │ 'steps' │ 4 │ 320 │ 'BLOB' │
│ Claiming external WEI (FeeToken: FRI) │ '1.512.000.000.256' │ 0.00615120000000004239 │ 2 │ 2 │ 'steps' │ 4 │ 256 │ 'BLOB' │
│ Claiming external WEI (FeeToken: FRI) │ '1.656.000.000.256' │ 0.006616560000000004643 │ 2 │ 2 │ 'steps' │ 4 │ 256 │ 'BLOB' │
│ Gifting FRI (FeeToken: WEI) │ '2.196.000.000.480' │ 0.0087 │ 2196000000000 │ 61 │ 52 │ 7 │ 3 │ 'steps' │ 5 │ 480 │ 'BLOB' │
│ Claiming FRI (FeeToken: WEI) │ '1.188.000.000.320' │ 0.0047 │ 1188000000000 │ 33 │ 30 │ 2 │ 2 │ 'steps' │ 4 │ 320 │ 'BLOB' │
│ Claiming external FRI (FeeToken: WEI) │ '1.512.000.000.320' │ 0.00615120000000004239 │ 2 │ 2 │ 'steps' │ 4 │ 320 │ 'BLOB' │
│ Claiming external FRI (FeeToken: WEI) │ '1.656.000.000.320' │ 0.006616560000000004643 │ 2 │ 2 │ 'steps' │ 4 │ 320 │ 'BLOB' │
│ Gifting FRI (FeeToken: FRI) │ '1.548.000.000.416' │ 0.0061 │ 1548000000000 │ 43 │ 37 │ 5 │ 2 │ 'steps' │ 4 │ 416 │ 'BLOB' │
│ Claiming FRI (FeeToken: FRI) │ '1.188.000.000.192' │ 0 │ 1188000000000 │ 33 │ 30 │ 2 │ 2 │ 'steps' │ 3 │ 192 │ 'BLOB' │
│ Claiming external FRI (FeeToken: FRI) │ '1.512.000.000.320' │ 0.00615120000000004239 │ 2 │ 2 │ 'steps' │ 4 │ 320 │ 'BLOB' │
│ Claiming external FRI (FeeToken: FRI) │ '1.656.000.000.320' │ 0.006616560000000004643 │ 2 │ 2 │ 'steps' │ 4 │ 320 │ 'BLOB' │
└───────────────────────────────────────┴─────────────────────┴─────────┴────────────────┴────────────────┴─────────────────┴───────────┴──────────────┴──────────────────────────────┴───────────────┴────────┴─────────┘
Resources:
┌───────────────────────────────────────┬─────────┬───────┬───────┬────────┬──────────┬──────────┬─────────────┬───────┐
Expand All @@ -25,14 +25,14 @@ Resources:
│ Transfer STRK (FeeToken: WEI) │ 0 │ 3 │ 0 │ 0 │ 25 │ 0 │ 181 │ 8184 │
│ Gifting WEI (FeeToken: WEI) │ 0 │ 3 │ 0 │ 0 │ 48 │ 0 │ 339 │ 14627 │
│ Claiming WEI (FeeToken: WEI) │ 0 │ 3 │ 0 │ 0 │ 47 │ 0 │ 373 │ 11715 │
│ Claiming external WEI (FeeToken: WEI) │ 0 │ 6 │ 0 │ 0 │ 52 │ 4 │ 42715434
│ Claiming external WEI (FeeToken: WEI) │ 0 │ 6 │ 0 │ 0 │ 52 │ 4 │ 48617007
│ Gifting WEI (FeeToken: FRI) │ 0 │ 3 │ 0 │ 0 │ 64 │ 0 │ 465 │ 20599 │
│ Claiming WEI (FeeToken: FRI) │ 0 │ 3 │ 0 │ 0 │ 47 │ 0 │ 407 │ 11913 │
│ Claiming external WEI (FeeToken: FRI) │ 0 │ 6 │ 0 │ 0 │ 52 │ 4 │ 42715434
│ Claiming external WEI (FeeToken: FRI) │ 0 │ 6 │ 0 │ 0 │ 52 │ 4 │ 48617007
│ Gifting FRI (FeeToken: WEI) │ 0 │ 3 │ 0 │ 0 │ 64 │ 0 │ 465 │ 20598 │
│ Claiming FRI (FeeToken: WEI) │ 0 │ 3 │ 0 │ 0 │ 47 │ 0 │ 373 │ 11715 │
│ Claiming external FRI (FeeToken: WEI) │ 0 │ 6 │ 0 │ 0 │ 52 │ 4 │ 42715434
│ Claiming external FRI (FeeToken: WEI) │ 0 │ 6 │ 0 │ 0 │ 52 │ 4 │ 48617007
│ Gifting FRI (FeeToken: FRI) │ 0 │ 3 │ 0 │ 0 │ 48 │ 0 │ 339 │ 14628 │
│ Claiming FRI (FeeToken: FRI) │ 0 │ 3 │ 0 │ 0 │ 47 │ 0 │ 407 │ 11913 │
│ Claiming external FRI (FeeToken: FRI) │ 0 │ 6 │ 0 │ 0 │ 52 │ 4 │ 42715434
│ Claiming external FRI (FeeToken: FRI) │ 0 │ 6 │ 0 │ 0 │ 52 │ 4 │ 48617007
└───────────────────────────────────────┴─────────┴───────┴───────┴────────┴──────────┴──────────┴─────────────┴───────┘
13 changes: 6 additions & 7 deletions src/contracts/claim_account.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@ mod ClaimAccount {
use core::ecdsa::check_ecdsa_signature;
use core::num::traits::Zero;
use starknet::{
TxInfo, account::Call, VALIDATED, syscalls::library_call_syscall, ContractAddress, get_contract_address,
get_execution_info, ClassHash
TxInfo, account::Call, VALIDATED, ContractAddress, get_contract_address, get_execution_info, ClassHash
};
use starknet_gifting::contracts::claim_account_impl::{
IClaimAccountImplLibraryDispatcher, IClaimAccountImplDispatcherTrait
Expand Down Expand Up @@ -114,12 +113,12 @@ mod ClaimAccount {

#[abi(embed_v0)]
impl GiftAccountImpl of IGiftAccount<ContractState> {
fn execute_action(ref self: ContractState, selector: felt252, calldata: Array<felt252>) -> Span<felt252> {
let mut calldata_span = calldata.span();
let claim: ClaimData = Serde::deserialize(ref calldata_span).expect('gift-acc/invalid-claim');
fn execute_action(ref self: ContractState, selector: felt252, args: Array<felt252>) -> Span<felt252> {
let mut args_copy = args.span();
let claim: ClaimData = Serde::deserialize(ref args_copy).expect('gift-acc/invalid-claim');
let implementation_class_hash = get_validated_impl(claim);
// TODO consider delegating to a fixed selector to we can have a whitelist of selectors in the implementation
library_call_syscall(implementation_class_hash, selector, calldata.span()).unwrap()
IClaimAccountImplLibraryDispatcher { class_hash: implementation_class_hash }
.execute_action(implementation_class_hash, selector, args)
}
}

Expand Down
22 changes: 18 additions & 4 deletions src/contracts/claim_account_impl.cairo
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use starknet::{ContractAddress};
use starknet::{ContractAddress, ClassHash};
use starknet_gifting::contracts::interface::{ClaimData, OutsideExecution, StarknetSignature};


Expand Down Expand Up @@ -31,6 +31,10 @@ pub trait IClaimAccountImpl<TContractState> {
fn execute_from_outside_v2(
ref self: TContractState, claim: ClaimData, outside_execution: OutsideExecution, signature: Span<felt252>
) -> Array<Span<felt252>>;

fn execute_action(
ref self: TContractState, this_class_hash: ClassHash, selector: felt252, args: Array<felt252>
) -> Span<felt252>;
}

#[starknet::contract]
Expand All @@ -40,9 +44,9 @@ mod ClaimAccountImpl {
use core::panic_with_felt252;
use openzeppelin::access::ownable::interface::{IOwnable, IOwnableDispatcherTrait, IOwnableDispatcher};
use openzeppelin::token::erc20::interface::{IERC20, IERC20DispatcherTrait, IERC20Dispatcher};
use starknet::{ClassHash, ContractAddress, get_caller_address, get_contract_address,};


use starknet::{
ClassHash, ContractAddress, get_caller_address, get_contract_address, syscalls::library_call_syscall
};
use starknet_gifting::contracts::claim_hash::{ClaimExternal, IOffChainMessageHashRev1};
use starknet_gifting::contracts::interface::{ClaimData, OutsideExecution, StarknetSignature};

Expand Down Expand Up @@ -82,6 +86,16 @@ mod ClaimAccountImpl {
array![]
}

fn execute_action(
ref self: ContractState, this_class_hash: ClassHash, selector: felt252, args: Array<felt252>
) -> Span<felt252> {
let whitelisted = selector == selector!("claim_external")
|| selector == selector!("get_dust")
|| selector == selector!("cancel");
assert(whitelisted, 'gift/invalid-selector');
library_call_syscall(this_class_hash, selector, args.span()).unwrap()
}

fn claim_external(
ref self: ContractState,
claim: ClaimData,
Expand Down
2 changes: 1 addition & 1 deletion src/contracts/interface.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ pub trait IGiftFactory<TContractState> {
#[starknet::interface]
pub trait IGiftAccount<TContractState> {
/// @notice delegates an action to the account implementation
fn execute_action(ref self: TContractState, selector: felt252, calldata: Array<felt252>) -> Span<felt252>;
fn execute_action(ref self: TContractState, selector: felt252, args: Array<felt252>) -> Span<felt252>;
}

/// @notice As defined in SNIP-9 https://github.com/starknet-io/SNIPs/blob/main/SNIPS/snip-9.md
Expand Down
Loading