Skip to content
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

Validate native denoms #17

Open
0xekez opened this issue Sep 1, 2022 · 2 comments
Open

Validate native denoms #17

0xekez opened this issue Sep 1, 2022 · 2 comments

Comments

@0xekez
Copy link

0xekez commented Sep 1, 2022

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)
}
@larry0x
Copy link
Contributor

larry0x commented Sep 1, 2022

this looks nice! actually i once tried using the regex for verification, but the regex library makes the contract binary too big to be put on-chain.

@0xekez
Copy link
Author

0xekez commented Sep 1, 2022

@larry0x: this looks nice!

Happy to put up a PR to add this. Are we early enough with versioning here that we can make a change that isn't backwards compatible?

Agree about regex. :) There are some ways to generate valid strings from a regex so I was thinking of trying to write some fuzzing code for this using that. There are some weird corners with go regex vs rust strings that I worry about: https://github.com/DA0-DA0/dao-contracts/blob/zeke/deposit-modules/packages/voting/src/denom.rs#L215-L223

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants