paritytech / polkadot-sdk

The Parity Polkadot Blockchain SDK
https://polkadot.com/
1.89k stars 697 forks source link

`with_genesis_config` not support `u128` type #4853

Closed AurevoirXavier closed 4 months ago

AurevoirXavier commented 4 months ago

with_genesis_config requires a serde_json::Value type. But it only supports up to u64.

But I think most chains are using u128 as their balance type.

https://substrate.stackexchange.com/questions/11100/number-out-of-range-on-configuring-balances/11522#11522

skunert commented 4 months ago

I think this was discussed before in https://github.com/paritytech/polkadot-sdk/issues/2963 and should have been fixed in https://github.com/paritytech/polkadot-sdk/pull/2987. @michalkucharczyk Have not looked deeply, but probably we should enable this feature in sc-chain-spec too?

AurevoirXavier commented 4 months ago

I think this was discussed before in #2963 and should have been fixed in #2987. @michalkucharczyk Have not looked deeply, but probably we should enable this feature in sc-chain-spec too?

Well, I don't think that's actually a fix on the user side. We still need to construct that JSON on our end and then pass it to the with_genesis_config function.

For instance, if people don't pay attention to this, they will likely fail at: https://github.com/paritytech/polkadot-sdk/blob/a23abb17232107275089040a33ff38e6a801e648/cumulus/polkadot-parachain/src/chain_spec/shell.rs#L41

michalkucharczyk commented 4 months ago

Add this feature (arbitrary_precision): https://github.com/serde-rs/json/blob/24d868f4e9be428afa1c744f8218a32660a1e0bf/Cargo.toml#L69-L75

here: https://github.com/XcavateBlockchain/Xcavate_Node/blob/2868ef7a53bd6640d32e7030f047fa58bef7d924/node/Cargo.toml#L20

This should fix the problem.

AurevoirXavier commented 4 months ago

This should fix the problem.

But should we leave this to the downstream?

What if make this function accept a D: DeserializeOwned or using function chain to add another patch like .with_genesis_config_patch(BalancesConfig { balances: vec![(.., ..)] }).

michalkucharczyk commented 4 months ago

From what I understand, the creation of JSON failed. Not deserialization on runtime side. (some unwrap that is inside json! macro).

michalkucharczyk commented 4 months ago

Basically this program fails (and this happens on node side):

[package]
name = "xxx"
version = "0.1.0"
edition = "2021"
[dependencies]
serde_json = {version = "1.0.117", default-features = false, features = ["alloc"] }
fn main() {
    serde_json::json!({
        "balances": {
            "balances": 1u128 << 112
        }
    });
}
michalkucharczyk commented 4 months ago

I think this was discussed before in #2963 and should have been fixed in #2987. @michalkucharczyk Have not looked deeply, but probably we should enable this feature in sc-chain-spec too?

You mean that features shall be merged by cargo? Yeah, we could add this, as sc-chain-spec is on the node side. But I am not 100% sure how features eventually will be resolved.

BTW: if this genesis config was defined on the runtime side (as preset) then this would not be a problem.

skunert commented 4 months ago

Basically this program fails (and this happens on node side):

Yeah was a misconception on my side. Building the JSON the responsibility of the downstream user of polkadot-SDK and easily fixed by arbitrary_precision. IMO we can close here.