Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Parent search for Aura should draw upon real asynchronous backing parameters #2706

Open
rphmeier opened this issue Jun 7, 2023 · 2 comments · May be fixed by #3059
Open

Parent search for Aura should draw upon real asynchronous backing parameters #2706

rphmeier opened this issue Jun 7, 2023 · 2 comments · May be fixed by #3059
Labels
I4-annoyance The node behaves within expectations, however this “expected behaviour” itself is at issue.

Comments

@rphmeier
Copy link
Contributor

rphmeier commented Jun 7, 2023

We should read the storage of the relay chain at the relay parent, but if it's not present fall back to a default hardcoded value of 0. Adding a utility function for reading the asynchronous backing parameters from the host configuration would be helpful for writing collators in the future.

async fn max_ancestry_lookback(
_relay_parent: PHash,
_relay_client: &impl RelayChainInterface,
) -> usize {
// TODO [https://github.com/paritytech/polkadot/pull/5022]
// We need to read the relay-chain state to know what the maximum
// age truly is, but that depends on those pallets existing.
//
// For now, just provide the conservative value of '2'.
// Overestimating can cause problems, as we'd be building on forks of the
// chain that can never get included. Underestimating is less of an issue.
2
}

@rphmeier rphmeier added the I4-annoyance The node behaves within expectations, however this “expected behaviour” itself is at issue. label Jun 7, 2023
@slumber
Copy link
Contributor

slumber commented Aug 22, 2023

I see 2 approaches we could follow there.

  1. Introduce async_backing_params method to RelayChainInterface and attempt to call into runtime API, defaulting to { 0, 0 } in case it's not available. Unfortunately, the error in this case is
ApiError(Application(Execution(Other("Exported method ParachainHost_staging_async_backing_params is not found"))))

Doing String::contains is ugly and overall is a bad practice.

  1. Use existing RelayChainInterface::get_storage_by_key, to read AbridgedHostConfiguration. Async backing parameters were placed directly under these fields (here), thus it should be safe, parameters were added in V6 migration on all chains. This diverges a little from what relay node does, but looks like a cleaner solution to me.

@rphmeier
Copy link
Contributor Author

Yes, I recommend approach (2). It will be more future-proof.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
I4-annoyance The node behaves within expectations, however this “expected behaviour” itself is at issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants