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

block_hash not completely usable? #86

Closed
dt665m opened this issue Apr 10, 2020 · 5 comments · Fixed by #87
Closed

block_hash not completely usable? #86

dt665m opened this issue Apr 10, 2020 · 5 comments · Fixed by #87

Comments

@dt665m
Copy link

dt665m commented Apr 10, 2020

@ascjones I could be just missing something obvious, but it seems the client's "async fn block_hash" is not completely usable by library users because "mod rpc" is private. The parameter is defined in mod rpc as pub type BlockNumber<T> = sp_rpc::number::NumberOrHex<<T as System>::BlockNumber>

Without reexporting sp_rpc::number::NumberOrHex or including rpc::BlockNumber in pub use, I'm not exactly sure how library users can get a specific block hash using a block number.

Incidentally, sp_rpc::number::NumberOrHex only implements std::convert::From for NumberOrHex<u64>, but kusama, node-template and basically all examples of substrate chains set System::BlockNumber to u32. Therefore,Some(9001u32.into()) does not work either.

Again, it is late and I might be missing something. If this is an oversight in the library, I can do a PR with some basic unit tests. I'm not sure if you would prefer to expose BlockNumber, reexport sp_rpc::number, or add more From's for different integer types in substrate/primitive/rpc.

Edit:

I was able to do sp_core::U256::from(x).into() to take advantage of the from Trait in sp_rpc::number. This works but seems to be a big circle to get to the right type.

Edit 2:

I just ran into the same issue with client.fetch for the StorageKey tuple struct. Can't call the method properly due to unexposed StorageKey. I'm importing my own substrate in Cargo.toml that is pinned to a specific git revision.

@ascjones
Copy link
Contributor

ascjones commented Apr 14, 2020

I could be just missing something obvious

No you're right this method is not very usable at the moment. The problem is personally I am using the library mainly for submitting extrinsics rather than querying which is why I haven't come across this.

Please take a look at #87 (just look at 62fdcb1 to ignore the code formatting) and let me know whether this helps.

I just ran into the same issue with client.fetch for the StorageKey tuple struct. Can't call the method properly due to unexposed StorageKey. I'm importing my own substrate in Cargo.toml that is pinned to a specific git revision.

Note my PR does not address this. Could simply reexport StorageKey but needs thinking about. Let me know what you think.

@dt665m
Copy link
Author

dt665m commented Apr 15, 2020

@ascjones thanks for addressing this so quickly! I know that for a general purpose client, you recommend substrate-api-client but for our purposes, this library is a better starting point. There is some form of metadata parsing already and I noticed you are also watching desub and it is async!

What I'm trying to do is to create an offchain event processor in rust and honestly it has been a lot more work than I expected. Most of the chain api effort seems to be placed on polkadotjs.

@ascjones
Copy link
Contributor

ascjones commented Apr 16, 2020

I'm considering extracting the general purpose rpc client to its own library, and just keeping the extrinsic specific stuff here.

Note that I'm currently working on scale-info for generating better metadata which should improve this story considerably. It would still need to be integrated into substrate (though that won't happen in the short term).

@dt665m
Copy link
Author

dt665m commented Apr 16, 2020

scale-info looks awesome. That should help when the substrate dev community grows into other languages. Everyone speaks json luckily(sadly?).

Since your simple metadata implementation is a good start for my purposes, do you mind if I extract/modify parts of it for now to get it to work for my event chaser? The metadata part in this crate is also private =)

@ascjones
Copy link
Contributor

Be my guest!

Also feel free to make a PR with any changes to visibility if you think it makes sense to have something as part of the public API.

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

Successfully merging a pull request may close this issue.

2 participants