Polkadot v0.9.37

[BillyWooo]

This is an update according to substrate (version polkadot-v0.9.37).
Big part of the update is still version bump up in toml files.
But @clangenb I am expecting some comment may come from you about the substrate-api-client part (mainly in commit: ab24686). I am crossing my fingers.

[clangenb]

Nice, thanks for the hard work of updating the api-client! I have one systematic change request here, which should improve the overall code because it removes the need for defining the Runtime in most cases.

When you apply my suggestion, be aware that you can also remove the my-node-runtime dependency in some cases.

[BillyWooo]

Nice, thanks for the hard work of updating the api-client! I have one systematic change request here, which should improve the overall code because it removes the need for defining the Runtime in most cases.

When you apply my suggestion, be aware that you can also remove the my-node-runtime dependency in some cases.

Hi @clangenb , yes I am expecting comments like this will come. About changes related to api trait, I had discussion with @haerdib and did some changes with her together. I don't have a chance to check carefully what's the difference of substrate-api-client between branch polkadot-v0.9.37 and master. Actually there are quite some changes.

If I understand right, the issue scs/substrate-api-client#406 you mentioned, should be implemented/refactored before we apply the api trait changes. What a pity I didn't see it before.

So here I can see 3 options:

• continue as it is, try to get the CI pass, then make the v0.9.37 done. (looks like you are not so satisfied with this approach 😅 )
• apply current changes of master to polkadot-v0.9.37 in substrate-api-client and fix/implement the issue you mentioned (clean up trait bound). Not sure how long it will takes. Actually substrate polkadot-v0.9.38 is coming soon.
• Rebase substrate-api-client 0.9.36, create new v0.9.37 branch (which means no native function/trait/etc. changes, only adapt the substrate upstream related change).

@clangenb @haerdib @Kailai-Wang What's your opinion?

[clangenb]

Sorry if I was not being clear. Yes, scs/substrate-api-client#406 will help to simplify the trait bounds here. However, my suggested change is unrelated to that. I only recommend removing the generic trait parameter from Chain<Runtime> and instead introduce associated types for the Hash, Balance and Index. If we do this, we don't need to import the runtime everywhere so, namely:

Before:

pub trait ChainApi<Runtime> {
   // some traitbounds...
   fn get_genesis_hash(&self) -> ApiResult<Runtime::Hash>
}

after

// This is actually the same pattern that the api-client already uses: https://github.com/scs/substrate-api-client/blob/07ed77d01c0d0205305e6bbff32525b53a5fbd05/src/extrinsic/balances.rs#L61
pub trait ChainApi {
      // no traitbounds needed in the trait declaration
      type Hash;

      fn get_genesis_hash(&self) -> ApiResult<Self::Hash>
}


impl<P: Pair, Client: RpcClient, Params, Runtime> ChainApi
	for Api<P, Client, Params, Runtime>
where
        // same traitbounds as with the other approach.
	MultiSignature: From<P::Signature>,
	Params: ExtrinsicParams<Runtime::Index, Runtime::Hash>,
	Runtime: BalancesConfig,
	Runtime::Hash: FromHexString + From<H256> + Into<H256>,
	Runtime::Index: Into<u32> + Decode,
	Runtime::Balance: TryFrom<NumberOrHex> + FromStr + Into<u128>,
{
   type Hash = Runtime::Hash

}

This in turn leads to the following difference:

before

use my_node_runtime::Runtime;

pub(crate) fn sync_state<
	E: TlsRemoteAttestation + EnclaveBase,
	NodeApi: PalletTeerexApi<Runtime>,
	WorkerModeProvider: ProvideWorkerMode,
>

after

// no runtime import needed.
// use my_node_runtime::Runtime;

pub(crate) fn sync_state<
	E: TlsRemoteAttestation + EnclaveBase,
	NodeApi: PalletTeerexApi,
	WorkerModeProvider: ProvideWorkerMode,
>

[clangenb]

Btw. sorry if you thought that I was dissatisfied. I think the PR is great except for my little change request. :)

[clangenb]

Thank you so much for the fixes, this looks exactly how I imagined it! Some minor comments remain and then we are good to go!

[clangenb]

Great, thanks a lot for the fixes! Will merge as soon as the CI passes.

[clangenb]

There seems to be a startup error in all the demos:

[2023-02-08T17:31:54Z WARN  itc_parentchain_light_client::io] could not backup previous light client state
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: InvalidHexString(InvalidHexCharacter { c: 'n', index: 0 })', service/src/main.rs:411:55
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

This is when it is trying to get the nonce from the chain.

[clangenb]

Closing in favor of this one: #1173

I am sorry for the extra effort you invested.

[BillyWooo]

Closing in favor of this one: #1173

I am sorry for the extra effort you invested.

Don't worry. Let's looking forward to the next upgrade.