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

Remove deprecated batch verification #13799

Merged

Conversation

bkchr
Copy link
Member

@bkchr bkchr commented Apr 1, 2023

This removes the deprecated batch verification. This was actually never really activated. Nevertheless, we need to keep the host functions around to support old runtimes which may import these host functions. However, we do not give access to these functions anymore. This means that any new runtime can not call them anymore. The host function implementations we keep will not do batch verification and will instead fall back to the always existing option of directly verifying the passed signature. finish_batch_verification will return the combined result of all the batch verify calls.

This removes the TaskExecutorExt which only existed to support the batch verification. So, any code that used this extension can just remove the registration of them. It also removes SignatureBatching that was used by frame-executive to control the batch verification. However, there wasn't any Verify implementation that called the batch verification functions.

polkadot companion: paritytech/polkadot#6999

This removes the deprecated batch verification. This was actually never really activated.
Nevertheless, we need to keep the host functions around to support old runtimes which may import
these host functions. However, we do not give access to these functions anymore. This means that any new
runtime can not call them anymore. The host function implementations we keep will not do batch verification and will
instead fall back to the always existing option of directly verifying the passed signature.
`finish_batch_verification` will return the combined result of all the batch verify calls.

This removes the `TaskExecutorExt` which only existed to support the batch verification. So, any
code that used this extension can just remove the registration of them. It also removes
`SignatureBatching` that was used by `frame-executive` to control the batch verification.
However, there wasn't any `Verify` implementation that called the batch verification functions.
@bkchr bkchr added A0-please_review Pull request needs code review. C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit B1-note_worthy Changes should be noted in the release notes F3-breaks_API This PR changes public API; next release should be major. T0-node This PR/Issue is related to the topic “node”. labels Apr 1, 2023
@bkchr bkchr requested a review from a team April 1, 2023 22:23
@ggwpez
Copy link
Member

ggwpez commented Apr 2, 2023

Nevertheless, we need to keep the host functions around to support old runtimes which may import these host functions

DQ but can they be feature gated since we mostly dont need them?

@davxy davxy requested a review from a team April 2, 2023 16:12
@bkchr
Copy link
Member Author

bkchr commented Apr 2, 2023

DQ but can they be feature gated since we mostly dont need them?

You can not use these functions anymore by calling sp_io::crypto::start_batch_verification. These are only given to the wasm executor to have them callable by old runtimes. Feature gating could work, but would currently be more complicated and doesn't bring any further value.

@bkchr
Copy link
Member Author

bkchr commented Apr 4, 2023

bot merge

@paritytech-processbot paritytech-processbot bot merged commit 842e651 into master Apr 4, 2023
@paritytech-processbot paritytech-processbot bot deleted the bkchr-remove-deprecated-batch-verification branch April 4, 2023 10:02
gpestana pushed a commit that referenced this pull request Apr 23, 2023
This removes the deprecated batch verification. This was actually never really activated.
Nevertheless, we need to keep the host functions around to support old runtimes which may import
these host functions. However, we do not give access to these functions anymore. This means that any new
runtime can not call them anymore. The host function implementations we keep will not do batch verification and will
instead fall back to the always existing option of directly verifying the passed signature.
`finish_batch_verification` will return the combined result of all the batch verify calls.

This removes the `TaskExecutorExt` which only existed to support the batch verification. So, any
code that used this extension can just remove the registration of them. It also removes
`SignatureBatching` that was used by `frame-executive` to control the batch verification.
However, there wasn't any `Verify` implementation that called the batch verification functions.
@Polkadot-Forum
Copy link

This pull request has been mentioned on Polkadot Forum. There might be relevant details there:

https://forum.polkadot.network/t/april-updates-for-substrate-and-polkadot-devs/2764/1

@Polkadot-Forum
Copy link

This pull request has been mentioned on Polkadot Forum. There might be relevant details there:

https://forum.polkadot.network/t/polkadot-release-analysis-v0-9-41-v0-9-42/2828/1

nathanwhit pushed a commit to nathanwhit/substrate that referenced this pull request Jul 19, 2023
This removes the deprecated batch verification. This was actually never really activated.
Nevertheless, we need to keep the host functions around to support old runtimes which may import
these host functions. However, we do not give access to these functions anymore. This means that any new
runtime can not call them anymore. The host function implementations we keep will not do batch verification and will
instead fall back to the always existing option of directly verifying the passed signature.
`finish_batch_verification` will return the combined result of all the batch verify calls.

This removes the `TaskExecutorExt` which only existed to support the batch verification. So, any
code that used this extension can just remove the registration of them. It also removes
`SignatureBatching` that was used by `frame-executive` to control the batch verification.
However, there wasn't any `Verify` implementation that called the batch verification functions.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B1-note_worthy Changes should be noted in the release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit F3-breaks_API This PR changes public API; next release should be major. T0-node This PR/Issue is related to the topic “node”.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants