Compact proof decoding fails when using the child trie

[yrong]

2021-09-03 11:47:24 panicked at 'Compact proof decoding failure.', /home/bifrost/.cargo/git/checkouts/cumulus-59522f43471fa161/fd80849/pallets/parachain-system/src/validate_block/implementation.rs:80:19 

After upgrade to v0.9.9 we've encounted this error several times when sending multiple ump transacts to relaychain. And workround is to hack in cumulus to disable compact

We can reproduce the issue and provide more logs if required, would be appreciated if anybody can help to trace and diagnose the root cause

[bkchr]

Sounds like your runtime isn't using the 0.9.9 branch yet?

[yrong]

Sounds like your runtime isn't using the 0.9.9 branch yet?

@bkchr I do not think so, you may check our cargo lock file

Btw, the call which triggers the error is (https://github.com/bifrost-finance/bifrost/blob/4e407dec25877805973a6e15cd3712fec11ff2ba/pallets/salp/src/lib.rs#L571)

Please help to check if anything wrong. And My question is any possibility that collator will fail to create compact proof in some circumstances?

[yrong]

@bkchr We find the cause. It will work after commenting these codes

fn put_contribution(
	index: TrieIndex,
	who: &AccountIdOf<T>,
	contributed: BalanceOf<T>,
	status: ContributionStatus<BalanceOf<T>>,
		) {
	who.using_encoded(|b| {
		child::put(&Self::id_from_index(index), b, &(contributed, status))
	});
}

So is the usage of child storages allowed in parachain?

[bkchr]

@cheme it seems that using the child trie breaks the compact proof?

[cheme]

Looks like it.
@yrong , do you have by any change a failing encoded compact proof (I don't think their is trace displaying it but maybe when you did your work around you did extract it somehow).

[yrong]

@cheme not sure fully catch what you mean. Maybe you can give some guidance about how to hack or produce detail logs which you think useful to debug.

Btw, child trie struct in our parachain side is just same as crowdloan module, I think you can reproduce easily if more than one entry of child_info stored

pub fn put<T: Encode>(child_info: &ChildInfo, key: &[u8], value: &T) {
	match child_info.child_type() {
		ChildType::ParentKeyId => value.using_encoded(|slice| {
			sp_io::default_child_storage::set(child_info.storage_key(), key, slice)
		}),
	}
}

[cheme]

By encoded proof I was thinking of a debug trace of the failing proof, for instance at bifrost-io@04262f9#r56045962.
I did find paritytech/substrate#9715 that can explain the issue, but with a failing compact proof sample, it will be safer.

[bkchr]

@yrong could you test the pr mentioned above?

[yrong]

@cheme verified and it works, thanks for the quick fix. @bkchr I guess parity would publish a patch and upgrade relaychain right? If so what is the next open window to upgrade kusama so we can arrange our release date accordingly.

[cheme]

Nice (actually the bug was my fault).
For this change the relay chain do not need upgrade, the decoding of the proof is done by the validation function of the cumulus parachain only, so it requires an update of the parachain validation function.

[bkchr]

@cheme verified and it works, thanks for the quick fix. @bkchr I guess parity would publish a patch and upgrade relaychain right? If so what is the next open window to upgrade kusama so we can arrange our release date accordingly.

We will include it in the 0.9.10 branch!