-
Notifications
You must be signed in to change notification settings - Fork 2
feat: add claim orders #3
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
base: main
Are you sure you want to change the base?
Conversation
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.
Some small things, otherwise LGTM
| self.orders.insert(&account_id, &vec![]); | ||
| } | ||
|
|
||
| let mut account_orders = self.orders.get(&account_id).unwrap(); |
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.
Expect with error message. Or like we do it in jars:
.unwrap_or_else(|| env::panic_str(&format!("Jar with id: '{}' doesn't exist", id.0)))
contract/src/order.rs
Outdated
| for account_id in account_ids { | ||
| let mut order_execution = OrderExecution::new(account_id.clone()); | ||
|
|
||
| let account_orders = self.orders.get(&account_id).expect("Account not found"); |
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.
[nit]
Add to error message which account was not found.
contract/src/order.rs
Outdated
| let account_orders = self.orders.get(&account_id).expect("Account not found"); | ||
| for order in account_orders { | ||
| let requested_amount = order.claim_amount.0; | ||
| let approved_amount = (requested_amount as f32 * percentage) as u128; |
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.
Is it ok that percentage here is f32? Shouldn't it be Decimal like in jars?
| } | ||
|
|
||
| self.internal_save_account_lockups(&order.account_id, account_lockup_indices); | ||
| // endregion |
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.
What is this?
// region cleanup
// endregion
| pub struct OrderExecution { | ||
| pub account_id: AccountId, | ||
| pub total_approved: Balance, | ||
| pub details: HashMap<LockupIndex, (Balance, Balance)>, // (Balance approved, Balance refund) |
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.
[nit]
Create struct so this comment isn't needed?
Change claim process to give a grant issuer priority right to buy tokens.