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

The ConstantsClient API needs better documentation #811

Closed
Tracked by #898
kylezs opened this issue Feb 3, 2023 · 3 comments
Closed
Tracked by #898

The ConstantsClient API needs better documentation #811

kylezs opened this issue Feb 3, 2023 · 3 comments

Comments

@kylezs
Copy link
Contributor

kylezs commented Feb 3, 2023

Maybe I'm using it wrong, in which case this issue could be to provide some documentation on how to actually get constants, but here's what I have.

let online_client = OnlineClient::<PolkadotConfig>::from_url("URL").await.unwrap();

let constant_client = ConstantsClient::<PolkadotConfig, _>::new(online_client).clone();

let block_hash_count: DecodedValue = constant_client
		.at::<StaticConstantAddress<DecodedValue>>(
			&StaticConstantAddress::<DecodedValue>::new("System", "BlockHashCount", [0u8; 32])
				.unvalidated()
				.into(),
		)
		.unwrap();

I would expect that I could just pass in a pallet, const name and a value (NOT this DecodedValue value type, but a u64 for example, and the client just returns me the value, decoded into the type I provide it, or something along these lines. Isn't clear at all how to use it, and I only got this far by reading the code, would be ideal if a user didn't, and could just use the API from the documentation provided.

One thing that I was confused about is why even have an object be passable into .at(), why not just pass in the generic, and do Address::pallet_name()? Seems that would be simpler.

@jsdw
Copy link
Collaborator

jsdw commented Feb 4, 2023

Offhand, it does seem like you are overcomplicating things a bit; perhaps this example will help?:

https://github.com/paritytech/subxt/blob/master/examples/examples/fetch_constants.rs

Assuming you have used the subxt macro to generate the various types, for your case you'd end up with something like:

// Create a client to use:
let api = OnlineClient::<PolkadotConfig>::new().await?;

// Build a constant address to query:
let address = polkadot::constants().system().block_hash_count();

// Look it up:
let block_hash_count = api.constants().at(&address)?;

One of the reasons for this API accepting concrete types is that you can also pass a dynamic "address" instead if you don't want to use the subxt macro (and from this you'd get back a "dynamic" Value type). This would look something like:

// Create a client to use:
let api = OnlineClient::<PolkadotConfig>::new().await?;

// Build a dynamic constant address instead of a static one to query:
let address = subxt::dynamic::constant("System", "BlockHashCount");

// Look it up (opting to decode the constant to a dynamic `Value` type; you can also manually work 
// with the bytes instead of using `to_value()`:
let block_hash_count = api.constants().at(&address)?.to_value()?;

I think offhand that the APIs are not too difficult to use, but the documentation is clearly lacking. We should:

  1. Add an example for dynamically decoding constants (I'd happily accept a PR for this). Ee have some other dynamic examples at https://github.com/paritytech/subxt/blob/master/examples/examples/dynamic_queries.rs but not constants for some reason, so it could be added there or in a separate example.
  2. Have a pass over the rust docs, because we could do with a bunch more examples alongside the code, and more guidance in the entry docs just to highlight these APIs better. Personally I find the "initial" docs not very useful (ie these ones: https://docs.rs/subxt/latest/subxt/) and think there should immediately be a bunch of examples on how to interact with things here, as well as with the individual functions.

I'm open to other suggestions also though!

@kylezs
Copy link
Contributor Author

kylezs commented Feb 6, 2023

Thanks for those examples, yes something like the second is what I was after. If I can get to a PR I will :)

Related, I guess there is some reason for only having DecodeValue be able to decode into only u128 and i128 and nothing smaller? Is this because any integer values will definitely fit into these and then it's up to the developer to cast smaller when they know it can fit?

@jsdw
Copy link
Collaborator

jsdw commented Feb 6, 2023

Thanks for those examples, yes something like the second is what I was after. If I can get to a PR I will :)

Related, I guess there is some reason for only having DecodeValue be able to decode into only u128 and i128 and nothing smaller? Is this because any integer values will definitely fit into these and then it's up to the developer to cast smaller when they know it can fit?

Yup pretty much; DecodedValue is just a generic representation of anything that can be scale decoded. To reduce complexity, it represents all numbers as i128 and u128, since they will all happily decode into these (and the goal is to be able to represent the decoded values accurately but not carry any specific "type" information).

It could (and did once) have variants for u8/u16 etc and there are arguments in favour of that too, but the size of the enum has to fit u128/i128 anyway, so it doesn't make the thing any smaller, and really just boils down to performance casting from/to 128 bit values versus implementation/usage complexity :)

You have the option to implement your own types that can be decoded to instead though if you prefer to roll your own custom type!

@jsdw jsdw changed the title The ConstantsClient API is too difficult to use The ConstantsClient API needs better documentation Feb 6, 2023
@jsdw jsdw closed this as completed Apr 6, 2023
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