I suggest to implement the cw-storage-plus PrimaryKey trait on AssetInfo directly instead of using an intermediary type AssetInfoKey. This allows the type to be easily used in maps and specifically multi-index maps.

Other than native and cw20. It would be good to support CW1155 multi-token contracts.

A CW1155 tokens consists of a contract_addr: Addr and a token_id: String

It could look something like:

#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)]
#[serde(rename_all = "snake_case")]
pub enum AssetInfoBase<T> {
    Cw20(T),
    Cw1155(T, String),
    Native(String),
}

Implement cw_storage_plus::PrimaryKey trait for AssetInfo, so that it can be used as storage keys, e.g.

use cosmwasm_std::Uint128;
use cw_storage_plus::Map;

const ASSET_AMOUNTS: Map<AssetInfo, Uint128> = Map::new("asset_amounts");

Cosmwasm 2.0 is out for a while, getting cosmwasm packages updated is a requirement for any project that depends on cw-asset to upgrade.

Currently, AssetInfo derives its PartialEq trait, meaning that comparing two AssetInfo::Cw20 instances will be a simple comparison of their contract addresses. This implies that two AssetInfo instances of the same CW20 token, but one with the address in lower case and the other in upper case, will be considered different:

let token1 = AssetInfo::cw20(api.addr_validate("terra1234abcd")?);
let token2 = AssetInfo::cw20(api.addr_validate("TERRA1234ABCD")?);
token1 == token2 // false

This could be a security vulnerability in some cases. Consider this situation: a contract holds some tokens. The contract owner implements a function to allow these tokens to be transferred out, but wants to withhold one from being transferred. A straightforward implementation would be:

const DISALLOWED_TOKEN: &str = "terra1234abcd";

fn execute_transfer(token_to_transfer: &Asset, recipient: &Addr) -> StdResult<CosmosMsg> {
    let disallowed_token_info = AssetInfo::cw20(api.addr_validate(DISALLOWED_TOKEN)?);
    if token_to_transfer.info == disallowed_token_info {
        return Err(StdError::generic_err("transfer of this token is not allowed"));
    }
    token_to_transfer.transfer_msg(recipient)
}

Since DISALLOWED_TOKEN is defined in lower case here, an exploiter can bypass the check by simply providing the token address in upper case.

Solution: When checking an AssetInfo instance, automatically converts all addresses to lower case.

Cosmos SDK stringifies native coins in a different format as cw-asset does. Whereas cw_asset::Asset strinfigifies to:

cw20:{address}:{amount}
native:{denom}:{amount}

SDK stringifies coins to:

{amount}{denom}

There should be a method AssetUnchecked::from_sdk_str to construct an AssetUnchecked::Native instance from the such strings.

We should be able to so this:

let info_unchecked = AssetInfoUnchecked::from_str("native:uatom")?;
let info = info_unchecked.check(api, None)?;

let asset_unchecked = AssetUnchecked::from_str("cw20:terra1a7zxk56c72elupp7p44hn4k94fsvavnhylhr6h:12345")?;
let asset = asset_unchecked.check(api, None)?;

let list_unchecked = AssetListUnchecked::from_str("native:uatom:12345,native:uluna:69420")?;
let list = list.unchecked.check(api, None)?;

Hi!

cw dependencies are getting old, could it be possible to update them from 0.13 to 0.16 on Cargo.toml?

Thanks!

Feel free to close this if you think this is a silly idea. I think we can implement transfer_from for native tokens easily enough by having it take a MessageInfo struct and then doing may_pay on it and returning no messages to be executed.

It's a bit of a break from the normal interface of this package, but I think semantically those are the same thing. For example:

let maybe_message = asset.transfer_from("ekez", info)?;

If ekez is the sender and the funds are included in the funds field it feels to me that the cw20 and native versions are the same here.

Based on the example at https://docs.rs/cw-asset/0.3.4/cw_asset/#legacy-feature, get error:

the trait bound Asset: From<AssetBase<cosmwasm_std::addresses::Addr>> is not satisfied
required because of the requirements on the impl of Into<Asset> for AssetBase<cosmwasm_std::addresses::Addr>

Cargo.toml

[package]
name = "test-assetinfo"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
cw-asset = { version = "0.3.3", features = ["legacy"] }
astroport = "1.0.1"

main.rs

fn main() {
    let a = cw_asset::Asset::native("uusd", 1u128);
    let b: astroport::asset::Asset = a.into();
}

I was recently looking into native denom validation, and it turns out there are actually some reasonably strict rules in the SDK as to what strings may be denominations. We should be able to perform that validation in a contract reasonably enough. From dao-contracts:

/// Follows cosmos SDK validation logic. Specifically, the regex
/// string `[a-zA-Z][a-zA-Z0-9/:._-]{2,127}`.
///
/// <https://github.com/cosmos/cosmos-sdk/blob/7728516abfab950dc7a9120caad4870f1f962df5/types/coin.go#L865-L867>
pub fn validate_native_denom(denom: String) -> Result<String, DenomError> {
    if denom.len() < 3 || denom.len() > 128 {
        return Err(DenomError::NativeDenomLength { len: denom.len() });
    }
    let mut chars = denom.chars();
    // Really this means that a non utf-8 character is in here, but
    // non-ascii is also correct.
    let first = chars.next().ok_or(DenomError::NonAlphabeticAscii)?;
    if !first.is_ascii_alphabetic() {
        return Err(DenomError::NonAlphabeticAscii);
    }

    for c in chars {
        if !(c.is_ascii_alphanumeric() || c == '/' || c == ':' || c == '.' || c == '_' || c == '-')
        {
            return Err(DenomError::InvalidCharacter { c });
        }
    }

    Ok(denom)
}

We can use AssetInfo or AssetInfoUnchecked as KeyDeserialize::Output for AssetInfoKey here to avoid unnecessary casting step.

impl KeyDeserialize for AssetInfoKey {
    type Output = Self;

    #[inline(always)]
    fn from_vec(value: Vec<u8>) -> StdResult<Self::Output> {
        Ok(Self(value))
    }
}

impl KeyDeserialize for AssetInfoKey {
    type Output = AssetInfoUnchecked;

    #[inline(always)]
    fn from_vec(value: Vec<u8>) -> StdResult<Self::Output> {
        Self::Output::try_from(Self(value))
    }
}

Since the addition of Cw1155 variants of AssetInfoBase, we should consider adding the #[non_exhaustive] macro tag to prevent breaking change of pattern matching when adding new enum variants.

Currently an empty AssetList stringifies to an empty string. Should output something more informative instead, such as [] or none