search

paritytech / polkadot-sdk

The Parity Polkadot Blockchain SDK
https://polkadot.network/
1.8k stars 652 forks source link

`CandidateValidationMessage::ValidateFromChainState` isn't used in production #5643

Open AndreiEres opened 2 weeks ago

AndreiEres commented 2 weeks ago

We use it in examples and malus. If we cut it, it can simplify the logic. cc @s0me0ne-unkn0wn

s0me0ne-unkn0wn commented 2 weeks ago

CC @ordian @pepyakin

I don't see any obstacles to removing it, but neither do I remember the reason for introducing it in the first place, so it would be good to have more eyes on it. I wonder when it was removed? Most probably it was used in disputes and maybe approvals. @alexggh @alindima do you remember removing it from there?

alexggh commented 2 weeks ago

@alexggh @alindima do you remember removing it from there

Nope, but I always thought it was added for malus.

ordian commented 2 weeks ago

It seems to be removed in the initial async backing impl, in particular https://github.com/paritytech/polkadot/pull/5557. I don't recall the reason for it though, but it seems like it's not really needed anymore for production.

sandreim commented 2 weeks ago

The way I understand it:

The only difference is that ValidateFromChainState also performs the runtime acceptance criteria checks, so it feels more like an optimization, but is redundant because in chain also verifies this so create inherent will filter any candidates that don't pass the criteria checks anyway.

I think it should be ok to remove it.