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

[BUG] kagami genesis synthetic's --assets-per-domain is actually --assets-per-account #3508

Open
0x009922 opened this issue May 21, 2023 · 1 comment
Labels
Bug Something isn't working good first issue Good for newcomers iroha2-dev The re-implementation of a BFT hyperledger in RUST

Comments

@0x009922
Copy link
Contributor

0x009922 commented May 21, 2023

GIT commit hash

5afcdef

Description

Kagami says that you can specify the amount of assets per domain, so the total amount of assets would be:

$$Assets = Domains * AssetsPerDomain$$

kagami help genesis synthetic
 --assets-per-domain <ASSETS_PER_DOMAIN>
          Number of assets per domains in synthetic genesis.
          Total number of assets would be `domains * assets_per_domain`
          
          [default: 0]

But if you look into implementation, you will see that actually it creates $$Assets = Domains * AccountsPerDomain * AssetsPerDomain$$

let mints = {
let mut acc = Vec::new();
for domain in 0..domains {
for account in 0..accounts_per_domain {
for asset in 0..assets_per_domain {
let mint = MintBox::new(
13_u32.to_value(),
IdBox::AssetId(AssetId::new(
format!("asset_{asset}#domain_{domain}").parse()?,
format!("account_{account}@domain_{domain}").parse()?,
)),
);
acc.push(mint);
}
}
}
acc
}

let mints = {
    let mut acc = Vec::new();
    for domain in 0..domains {
        for account in 0..accounts_per_domain {
            for asset in 0..assets_per_domain {
                let mint = MintBox::new(
                    13_u32.to_value(),
                    IdBox::AssetId(AssetId::new(
                        format!("asset_{asset}#domain_{domain}").parse()?,
                        format!("account_{account}@domain_{domain}").parse()?,
                    )),
                );
                acc.push(mint);
            }
        }
    }
    acc
}

So, currently this option is actually not --assets-per-domain, but --assets-per-account.

Expected result

Expected either to have $$Assets = Domains * AssetsPerDomain$$ or to have --assets-per-account option instead.

@0x009922 0x009922 added Bug Something isn't working iroha2-dev The re-implementation of a BFT hyperledger in RUST labels May 21, 2023
@Erigara
Copy link
Contributor

Erigara commented Aug 12, 2023

@0x009922 currently we have assets definitions per domain, should we rename asset_definitions_per_domain?

@mversic mversic added the good first issue Good for newcomers label Nov 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working good first issue Good for newcomers iroha2-dev The re-implementation of a BFT hyperledger in RUST
Projects
None yet
Development

No branches or pull requests

3 participants