-
Notifications
You must be signed in to change notification settings - Fork 0
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
Fix small remarks #3
Merged
Merged
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
b6b1ef6
first draft
gaetbout a1265af
rename max-fee error message
gaetbout 853df7a
remove test scripts
gaetbout 96769e4
move compute max fee back onto the account
gaetbout 7349058
fix prev push
gaetbout 0f7f6b1
remove extra field in ClaimExternal
gaetbout 5cd716e
remove extra field in ClaimExternal
gaetbout 5e209f5
read class_hash in var
gaetbout d48573d
get_claim_address takes class_hash as param
gaetbout 72c645d
read fn should use @
gaetbout a5e599d
remove extra line
gaetbout 4f55eab
rename fn to get_latest_claim_class_hash
gaetbout 573332c
fix cairo tests
gaetbout af31b52
fixing tests
gaetbout File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Oops, something went wrong.
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -102,16 +102,15 @@ mod GiftFactory { | |
) { | ||
self.pausable.assert_not_paused(); | ||
assert(token == STRK_ADDRESS() || token == ETH_ADDRESS(), 'gift-fac/invalid-token'); | ||
// TODO: Assert max_fee is not zero? | ||
assert(max_fee.into() < amount, 'gift-fac/fee-too-high'); | ||
|
||
let sender = get_caller_address(); | ||
let factory = get_contract_address(); | ||
// TODO We could manually serialize for better performance | ||
// TODO pubkey can be zero? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is solved ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
let class_hash = self.claim_class_hash.read(); | ||
let constructor_arguments = AccountConstructorArguments { sender, amount, max_fee, token, claim_pubkey }; | ||
let (claim_contract, _) = deploy_syscall( | ||
self.claim_class_hash.read(), // class_hash | ||
class_hash, // class_hash | ||
0, // salt | ||
serialize(@constructor_arguments).span(), // constructor data | ||
false // deploy_from_zero | ||
|
@@ -120,14 +119,7 @@ mod GiftFactory { | |
self | ||
.emit( | ||
GiftCreated { | ||
claim_pubkey, | ||
factory, | ||
gift_address: claim_contract, | ||
class_hash: self.claim_class_hash.read(), | ||
sender, | ||
amount, | ||
max_fee, | ||
token, | ||
claim_pubkey, factory, gift_address: claim_contract, class_hash, sender, amount, max_fee, token, | ||
} | ||
); | ||
let transfer_status = IERC20Dispatcher { contract_address: token } | ||
|
@@ -139,7 +131,7 @@ mod GiftFactory { | |
let claim_address = self.check_factory_and_get_account_address(claim); | ||
assert(get_caller_address() == claim_address, 'gift/only-claim-account'); | ||
let balance = IERC20Dispatcher { contract_address: claim.token }.balance_of(claim_address); | ||
assert(balance > claim.amount, 'gift-acc/gift-canceled'); | ||
assert(balance >= claim.amount, 'gift/already-claimed-or-cancel'); | ||
self.transfer_from_account(claim, claim_address, claim.token, claim.amount, receiver); | ||
self.emit(GiftClaimed { receiver }); | ||
} | ||
|
@@ -148,25 +140,26 @@ mod GiftFactory { | |
ref self: ContractState, claim: ClaimData, receiver: ContractAddress, signature: Array<felt252> | ||
) { | ||
let claim_address = self.check_factory_and_get_account_address(claim); | ||
let claim_external_hash = ClaimExternal { claim, receiver }.get_message_hash_rev_1(claim_address); | ||
let claim_external_hash = ClaimExternal { receiver }.get_message_hash_rev_1(claim_address); | ||
assert( | ||
check_ecdsa_signature(claim_external_hash, claim.claim_pubkey, *signature[0], *signature[1]), | ||
'gift-acc/invalid-ext-signature' | ||
'gift/invalid-ext-signature' | ||
); | ||
|
||
let balance = IERC20Dispatcher { contract_address: claim.token }.balance_of(claim_address); | ||
assert(balance >= claim.amount, 'gift/already-claimed-or-cancel'); | ||
self.transfer_from_account(claim, claim_address, claim.token, balance, receiver); | ||
self.emit(GiftClaimed { receiver }); | ||
} | ||
|
||
fn cancel(ref self: ContractState, claim: ClaimData) { | ||
let claim_address = self.check_factory_and_get_account_address(claim); | ||
assert(get_caller_address() == claim.sender, 'gift-acc/wrong-sender'); | ||
assert(get_caller_address() == claim.sender, 'gift/wrong-sender'); | ||
|
||
let balance = IERC20Dispatcher { contract_address: claim.token }.balance_of(claim_address); | ||
// Won't that lead to the sender also being able to get the extra dust? | ||
// assert(balance > claim.max_fee, 'already claimed'); | ||
assert(balance > 0, 'gift-acc/already-claimed'); | ||
assert(balance > 0, 'gift/already-claimed'); | ||
self.transfer_from_account(claim, claim_address, claim.token, balance, claim.sender); | ||
self.emit(GiftCanceled {}); | ||
} | ||
|
@@ -181,28 +174,21 @@ mod GiftFactory { | |
self.transfer_from_account(claim, claim_address, claim.token, balance, receiver); | ||
} | ||
|
||
fn get_claim_class_hash(ref self: ContractState) -> ClassHash { | ||
fn get_latest_claim_class_hash(self: @ContractState) -> ClassHash { | ||
self.claim_class_hash.read() | ||
} | ||
|
||
fn get_claim_address( | ||
self: @ContractState, | ||
class_hash: ClassHash, | ||
sender: ContractAddress, | ||
amount: u256, | ||
max_fee: u128, | ||
token: ContractAddress, | ||
claim_pubkey: felt252 | ||
) -> ContractAddress { | ||
calculate_claim_account_address( | ||
ClaimData { | ||
factory: get_contract_address(), | ||
class_hash: self.claim_class_hash.read(), | ||
sender, | ||
amount, | ||
max_fee, | ||
token, | ||
claim_pubkey, | ||
} | ||
ClaimData { factory: get_contract_address(), class_hash, sender, amount, max_fee, token, claim_pubkey, } | ||
) | ||
} | ||
} | ||
|
@@ -254,7 +240,7 @@ mod GiftFactory { | |
] | ||
); | ||
let transfer_status = full_deserialize::<bool>(*results.at(0)).expect('gift/invalid-result-calldata'); | ||
assert(transfer_status, 'gift-acc/transfer-failed'); | ||
assert(transfer_status, 'gift/transfer-failed'); | ||
} | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why move from the utils ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thought it can be part of the account now that we reduced its code and doesn't need to be shared anymore
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agree it belongs here