Mohit/currency manager #20
Conversation
|
@Mohiiit remove all the test we will open separate issues for those |
|
@and1vo can you approve the workflows for this PR? |
Approved! |
Mohiiit
force-pushed
the
mohit/currency_manager
branch
from
fa7f6c5 to
c6ee00e
Compare
Mohiiit
force-pushed
the
mohit/currency_manager
branch
from
8127aae to
5cddb1b
Compare
yes, I will solve the merge conflicts, I thought once the upgradable doubt is resolved, I will solve the merge conflicts |
|
2 more thing:
After that it LGTM! |
Mohiiit
force-pushed
the
mohit/currency_manager
branch
from
17c21f4 to
10aefb2
Compare
Mohiiit
force-pushed
the
mohit/currency_manager
branch
from
3047fb9 to
d8b1ad9
Compare
| fn is_currency_whitelisted(self: @TState, currency: ContractAddress) -> bool; | ||
| fn whitelisted_currency_count(self: @TState) -> usize; | ||
| fn whitelisted_currency(self: @TState, index: usize) -> ContractAddress; | ||
| } | ||
|
|
||
| #[starknet::interface] | ||
| trait ICurrencyManage<TState> { |
There was a problem hiding this comment.
should be ICurruncyManager
There was a problem hiding this comment.
It would be same as the interface just above, we can't have that
There was a problem hiding this comment.
oh i see what is the issue, you can remove completely the interface with Ownable functions, we will use IOwnable interface for testing. after that LGTM let's just wait for the workflow to be approved (not sure why it does still need approval). Also if you have problem with assert! formatting ContractAddress my last PR will fix that once merged you'll just have to import flex::{Debug, Display}; in your contract
There was a problem hiding this comment.
Yes, it's failing because of the assert! formatting, I will pull and merge those changes, once they are merged.
There was a problem hiding this comment.
PR that fixes your issue is merged
There was a problem hiding this comment.
@Mohiiit Hey man, any updates on this PR!
There was a problem hiding this comment.
Hey @and1vo, Yes I will update this branch, with the latest changes in place. was away for sometime, will do it asap
There was a problem hiding this comment.
hey @Mohiiit just checkin in!
|
I will create another pull request soon this week. |
onwerandtransfer_ownershipbecause they will clash with the ownable component we are using