fix: SignedPhase is removed from metadata #803
Conversation
Because the `SignedPhase` is removed from the metadata we have no good alternativ than to hardcode this constants for now.
| Default::default(), | ||
| )?; | ||
| let params = DefaultExtrinsicParamsBuilder::new().nonce(nonce).build(); | ||
| let xt = client.chain_api().tx().create_signed(&tx, &*signer, params).await?; |
| @@ -209,85 +208,49 @@ async fn run_command( | |||
| } | |||
|
|
|||
| /// Runs until the RPC connection fails or updating the metadata failed. | |||
| async fn runtime_upgrade_task(client: Client, tx: oneshot::Sender<Error>, mut spec_version: u32) { | |||
There was a problem hiding this comment.
this bug has been fixed in subxt and we can rely on that now :)
| @@ -9,8 +9,6 @@ pub struct Client { | |||
| rpc: RpcClient, | |||
| /// Access to chain APIs such as storage, events etc. | |||
| chain_api: ChainClient, | |||
| /// Raw RPC client. | |||
| raw_rpc: RawRpcClient, | |||
| ) -> Result<(), Error> { | ||
| let tx = epm::signed_solution(RawSolution { solution, score, round })?; | ||
|
|
||
| // TODO: https://github.com/paritytech/polkadot-staking-miner/issues/730 | ||
| // | ||
| // The extrinsic mortality length is static and doesn't know when the | ||
| // signed phase ends. | ||
| let signed_phase_len = client |
There was a problem hiding this comment.
Just to double check: Instead of fetching the signed phase constant from the chain, we define our own signed phase in the static types.rs. And this is because the constant from the chain wasn't what we'd expect?
There was a problem hiding this comment.
from the PR description: its because the SignedPhase is removed from westend in preparation for the merkalized metadata.
There was a problem hiding this comment.
the SignedPhase is removed from the metadata and it will impact the other chains as well when they update the EPM pallet
src/static_types.rs
Outdated
| @@ -99,6 +99,9 @@ pub mod westend { | |||
| >(16) | |||
| ); | |||
|
|
|||
| // SYNC https://github.com/paritytech/polkadot-sdk/blob/master/polkadot/runtime/westend/src/lib.rs#L451-#L453. | |||
There was a problem hiding this comment.
A crazy idea would be to have a macro which keeps this in sync at compile time or a CI job that fetches that value, but it sounds a bit too complicated since the signed phase depends on multiple constants defined in substrate 🤔
src/static_types.rs
Outdated
| @@ -99,6 +99,9 @@ pub mod westend { | |||
| >(16) | |||
| ); | |||
|
|
|||
| // SYNC https://github.com/paritytech/polkadot-sdk/blob/master/polkadot/runtime/westend/src/lib.rs#L451-#L453. | |||
| pub const SIGNED_PHASE_LENGTH: u64 = 150; | |||
There was a problem hiding this comment.
This is a temp fix but I wonder if there are any ideas to make sure this value is up to date throughout all chains? or should we just keep a sensible upper bound since this is just used for the tx mortality?
If we leave it as is, I'd suggest us to rename SIGNED_PHASE_LENGTH to SIGNED_PHASE_LENGTH_ESTIMATE or something similar so that the const is now mistakenly used in the future.
There was a problem hiding this comment.
it's removed now and I changed it to use babe::epoch_duration as upper bound/tx mortality ok?
Because the
constant SignedPhaseis removed from the metadata we have no good alternative than to hardcode it for now.The sign phase length is only used to configure the mortality of the submit solution transaction so not super critical if it's slighty off.
In addition I fetched the updated metadata from polkadot master in order to indicate that the signed phase is not exposed in the metadata anymore and updated some polkadot-sdk deps. (this will not break compatibility with older versions)