check_inherents method should be in the CoreApi, not the BlockBuilderApi.

[JoshOrndorff]

My understanding is that the BlockBuilderApi is for the process of authoring block, while the CoreApi is for the more fundamental job of importing and checking blocks.

All nodes should check the inherents on newly imported blocks whether they are authors or not, and therefore, the check_inherents method should be moved to the CoreApi.

[bkchr]

All nodes should check the inherents on newly imported blocks whether they are authors or not,

Actually only validators need to do so. Normal full nodes would not need to do this. Given this, I don't see any reason for moving the function.

[JoshOrndorff]

If it is only the validators who need to check inherents, then I agree the function should not move.

I was surprised to hear that only validators need to check them though. Would you say that checking inherents is less important or fundamental than checking the execution of transactions? And why?

[bkchr]

Would you say that checking inherents is less important

It is basically an additional job that needs to be done. And if the validators agree that the inherents are not correct, they will not build on top of this block and thus, the block will "disappear".

fundamental than checking the execution of transactions

If we could, we would probably skip the execution at all and only do it on validators, same argument as above. However, we need to re-run all transactions to get the state changes.

[kayabaNerve]

Only having validators check transactions, inherent or otherwise, removes public verifiability of the blockchain. It also enables validators to perform illegal operations outside of equivocation. IMO, that fundamentally breaks the security model of running your own. To so trust validators means you might as well run a light client and gain the efficiency while you're at it.

Not only do I want to agree with the original post, I want to ask for clarification:

In the current Substrate, are inherents only checked so long as the node is actively a block author?

If so, this would've fundamentally broken prior designs I had and caused the complete collapse of my project (again, if I had moved forward with those designs). While that may be arguable as my own bad design, it creates a commentary trusted inherents are a footgun.

[bkchr]

In the current Substrate, are inherents only checked so long as the node is actively a block author?

Currently all nodes are running this.

Only having validators check transactions, inherent or otherwise, removes public verifiability of the blockchain. It also enables validators to perform illegal operations outside of equivocation.

You are mixing here a lot of things together. Inherents are any way just things that you pass to the runtime that the runtime can not verify. Things like checking that a timestamp is not 3000 years it not really a check that needs to be done by every node. If >2/3 of your validator set would agree to such a block, you have bigger problems as your validator set is just corrupt or whatever. If you sync the chain, you can not and will never be able to verify if a timestamp was correct around when the block was produced.

In general, you should check your assumptions and ensure that you don't do weird things there.

If so, this would've fundamentally broken prior designs I had and caused the complete collapse of my project (again, if I had moved forward with those designs). While that may be arguable as my own bad design, it creates a commentary trusted inherents are a footgun.

This sounds like a red flag and you should re-check your assumptions.

[kayabaNerve]

1. Inherents may not be completely reproducible, yet they can still be verifiable to be within tolerances (as validators do). I'll also note while you may argue validator set corruption is the larger issue, that doesn't change the impact reachable upon corruption increases by this vector (which may have effects. I use the time contained within Substrate as a consensus-agreed upon time, almost like a deterministic NTP. The lack of full node checks greatly increases how wrong said time can be).

2. I had a write-up on why this was a documentation failure, yet the current Rust docs ProvideInherent seems fine. I'd guess it was written after I read it (which was a while back), or I must have not read it (as I was frequently relying on guides such as the ones https://docs.substrate.io at the time).

I'll withdraw my commentary on Inherent behavior being a 'problem' due to my second reason however, even though I still think better behavior could/should exist, and apologize for contributing my confusions there to this issue.

[bkchr]

1. I use the time contained within Substrate as a consensus-agreed upon time, almost like a deterministic NTP. The lack of full node checks greatly increases how wrong said time can be).

The point being that full nodes could not change anything, they would may reject a block, but the validators would continue to produce blocks. You don't win anything from full nodes checking the inherents.

even though I still think better behavior could/should exist, and apologize for contributing my confusions there to this issue.

I mean if your implementation requires it, you can still check the inherents. There is nothing that speaks against this. However, as said above, full nodes will not contribute to any better security.

[kayabaNerve]

That assumes that the only worthwhile actions happen on-chain. If full nodes reject bad timestamp inherents, then those nodes' RPCs won't offer data premised on blocks with bad timestamps. That is the potentially impactful benefit.

[bkchr]

That assumes that the only worthwhile actions happen on-chain.

I mean this is the entire purpose of the machinery we are building here ;) Make stuff verifyable and not requiring any trust on random nodes.

If full nodes reject bad timestamp inherents, then those nodes' RPCs won't offer data premised on blocks with bad timestamps

Full nodes are also any way a concept of the past. Light clients for sure will never check inherents and thus, they will just trust these blocks.

[kayabaNerve]

I agree with the ethos ;) Though my comment here is you're applying an, IMO limited, scope.

For context, my project has validators for its Substrate-chain which acts as a canonical clock (as in event flow, not as time) and orderer for a variety of unsynchronized event streams. Each independent event stream has its own designated set of validators who need to and do run full nodes (as otherwise compromising the Substrate validators would compromise all event streams). Accordingly, they're not validators per Substrate, yet they are validators for the project as a whole. The system as a whole does achieve verifiability, yet with a notable amount of work performed off-Substrate.

My work doesn't collapse if the timestamp is forged. It'll cause a halt IIRC, which'd happen if the validators simply went offline. My advocacy for time, and inherents in general, to be checked in general is to reduce surface scope and potential impact reach (both in my project and in general).

I'd also point out that yes, light clients are the way to access the chain without full checks and execution. I have no qualms with them not checking inherents :p I disagree with "full" nodes not "fully" checking inherents as possible but I'd understand if inherents as currently used in-ecosystem require this behavior and if Substrate (as an ecosystem) disagrees with my personal opinion which only really stands with the one consistent example used here of the timestamp pallet.

Thanks for talking this through with me :)