v2 #11
v2 #11
Conversation
…adata Allow custom LP subdenom and setting denom metadata
|
|
||
| /// get the alloyed denom | ||
| pub fn get_alloyed_denom(&self, store: &dyn Storage) -> StdResult<String> { | ||
| self.alloyed_denom.load(store) |
There was a problem hiding this comment.
I generally like this pattern, but I'm thinking that it may lead to loads happening multiple times if the caller uses several get methods.
Not release-blocker, but we can consider caching these (if the data is there return it, otherwise fetch)
There was a problem hiding this comment.
(just remember to reset the cache on sets! :p )
contracts/transmuter/src/contract.rs
Outdated
| @@ -128,13 +346,44 @@ impl Transmuter<'_> { | |||
| &self, | |||
| ctx: (DepsMut, Env, MessageInfo), | |||
| tokens_out: Vec<Coin>, | |||
| ) -> Result<Response, ContractError> { | |||
| self.swap_alloyed_asset_for_tokens("exit_pool", ctx, tokens_out) | |||
There was a problem hiding this comment.
These being join/exit pool confused me at first, but I get it now
contracts/transmuter/src/contract.rs
Outdated
| let alloyed_denom = self.alloyed_asset.get_alloyed_denom(deps.storage)?; | ||
|
|
||
| // if funds contains one coin and is alloyed asset, use that as sender's share | ||
| let (sender_shares, burn_from_address) = if info.funds.is_empty() { |
There was a problem hiding this comment.
Can you explain these two blocks of code?
There was a problem hiding this comment.
ohh, this is what makes it work as both exit_pool and swap. Maybe just add some comments for each block
contracts/transmuter/src/contract.rs
Outdated
| // --- admin --- | ||
|
|
||
| #[msg(exec)] | ||
| pub fn transfer_admin( |
There was a problem hiding this comment.
Not transmuter specific, but there are a few patterns that we keep using. It would be good to make sylvia interfaces that we can reuse on all osmosis contracts
There was a problem hiding this comment.
Good idea, I think this is simlilar to openzeppelin Ownable, we might wanna have our own collection.
There was a problem hiding this comment.
100%! we could even add some osmosis-specific stuff behind compilation flags (so they're only included when needed)
V2 issue tracker: #5
included PRs