From 213be790e798a745e2ee5e6e28587b4238ac7263 Mon Sep 17 00:00:00 2001 From: Haiko Schol Date: Thu, 12 Dec 2024 20:44:28 +0700 Subject: [PATCH 1/5] docs(parachain): add design for bitfield signing --- docs/docs/design/bitfield-signing.md | 90 ++++++++++++++++++++++++++++ 1 file changed, 90 insertions(+) create mode 100644 docs/docs/design/bitfield-signing.md diff --git a/docs/docs/design/bitfield-signing.md b/docs/docs/design/bitfield-signing.md new file mode 100644 index 0000000000..c539dcc632 --- /dev/null +++ b/docs/docs/design/bitfield-signing.md @@ -0,0 +1,90 @@ +# Bitfield Signing Subsystem + +[The bitfield signing subsystem](https://paritytech.github.io/polkadot-sdk/book/node/availability/bitfield-signing.html) +is responsible for producing signed availability bitfields and passing them to +[the bitfield distribution subsystem](./bitfield-distribution.md). It only does that when running on a validator. + +## Subsystem Structure + +The implementation must conform to the `Subsystem` interface defined in the `parachaintypes` package. It should live in +a package named `bitfieldsigning` under `dot/parachain/bitfield-signing`. + +### Messages Received + +The subsystem must be registered with the overseer. It only handles `parachaintypes.ActiveLeavesUpdateSignal`. The +overseer must be modified to forward this message to the subsystem. + +### Messages Sent + +1. [`DistributeBitfield`](https://github.com/paritytech/polkadot-sdk/blob/1e3b8e1639c1cf784eabf0a9afcab1f3987e0ca4/polkadot/node/subsystem-types/src/messages.rs#L522) + +```go +package parachaintypes + +import "github.com/ChainSafe/gossamer/lib/common" + +type DistributeBitfield struct { + RelayParent common.Hash + Bitfield UncheckedSignedAvailabilityBitfield +} +``` + +As mentioned in [bitfield distribution](./bitfield-distribution.md), it makes sense to duplicate the type +`UncheckedSignedAvailabilityBitfield` as `CheckedSignedAvailabilityBitfield` or just `SignedAvailabilityBitfield` and +use it in `DistributeBitfield`. + +2. [`AvailabilityStore::QueryChunkAvailability(CandidateHash, ValidatorIndex, response_channel)`](https://github.com/paritytech/polkadot-sdk/blob/1e3b8e1639c1cf784eabf0a9afcab1f3987e0ca4/polkadot/node/subsystem-types/src/messages.rs#L556) + +This type is already defined as `availabilitystore.QueryChunkAvailability` in +`dot/parachain/availability-store/messages.go`. + +## Subsystem State + +The subsystem needs access to the parachain session keys for signing bitfields. The appropriate `Keystore` should be +passed to the subsystem constructor as a dependency to facilitate testing. Since each new active leave results in a +goroutine being spawned, the subsystem needs to maintain state to receive data from them and to cancel them when +appropriate. + +```go +package bitfieldsigning + +import ( + "context" + + "github.com/ChainSafe/gossamer/lib/common" + "github.com/ChainSafe/gossamer/lib/keystore" +) + +type signingTask struct { + ctx context.Context + response <-chan parachaintypes.UncheckedSignedAvailabilityBitfield +} + +type BitfieldSigning struct { + keystore keystore.Keystore + tasks map[common.Hash]*signingTask +} +``` + +Again, this should probably use a (to be created) type `CheckedSignedAvailabilityBitfield`. + +## Message Handling Logic + +### `parachaintypes.ActiveLeavesUpdateSignal` + +For all leaves that have been deactivated, cancel the signing tasks associated with them. + +For all activated leaves, spawn a new signing task. The task needs to perform the following steps: + +1. Wait a while to let the availability store get populated with information about erasure chunks. In the Parity node, +this delay is currently set to [1500ms](https://github.com/paritytech/polkadot-sdk/blob/1e3b8e1639c1cf784eabf0a9afcab1f3987e0ca4/polkadot/node/core/bitfield-signing/src/lib.rs#L49). + +2. Query the set of [availability cores](https://paritytech.github.io/polkadot-sdk/book/runtime-api/availability-cores.html) +for the given leaf from the runtime. + +3. For each core, concurrently check whether the core is occupied and if so, query the availability store using +`QueryChunkAvailability`. + +4. Collect the results of the queries into a bitfield, sign it and send it to the subsystem. + +5. In the subsystem, receive the signed bitfield and send a `DistributeBitfield` message to the overseer. From 0979adeab9778cbe96286b19e553c83022f21ea8 Mon Sep 17 00:00:00 2001 From: Haiko Schol Date: Thu, 12 Dec 2024 11:23:14 +0700 Subject: [PATCH 2/5] docs(parachain): move DistributeBitfield to parachaintypes --- docs/docs/design/bitfield-distribution.md | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/docs/docs/design/bitfield-distribution.md b/docs/docs/design/bitfield-distribution.md index 2dfb547b7f..87fc1249f2 100644 --- a/docs/docs/design/bitfield-distribution.md +++ b/docs/docs/design/bitfield-distribution.md @@ -28,16 +28,13 @@ network message as part of the message handling code. subsystem.) ```go -package bitfielddistribution +package parachaintypes -import ( - parachaintypes "github.com/ChainSafe/gossamer/dot/parachain/types" - "github.com/ChainSafe/gossamer/lib/common" -) +import "github.com/ChainSafe/gossamer/lib/common" type DistributeBitfield struct { RelayParent common.Hash - Bitfield parachaintypes.UncheckedSignedAvailabilityBitfield + Bitfield UncheckedSignedAvailabilityBitfield } ``` From 1ccf499718fcfe036b2a6187fe3c67b9a608530d Mon Sep 17 00:00:00 2001 From: Haiko Schol Date: Fri, 13 Dec 2024 16:48:24 +0700 Subject: [PATCH 3/5] link to bitfield signing doc from availability subsystems --- docs/docs/design/availability-subsystems.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/docs/design/availability-subsystems.md b/docs/docs/design/availability-subsystems.md index e15d593f7c..48db41df13 100644 --- a/docs/docs/design/availability-subsystems.md +++ b/docs/docs/design/availability-subsystems.md @@ -28,7 +28,7 @@ Availability is handled by these subsystems: - Requests chunks from backing validators to put them in their local Availability Store. - Validator with index i gets chunk with the same index. -## Bitfield Signing subsystem +## [Bitfield Signing subsystem](./bitfield-signing.md) - For each fresh leaf, wait a fixed period of time for availability distribution to make candidate available. - Validator with index i gets a chunk with the same index. From dd1ebdf63604acfac2a844e9ba99e31aa956576c Mon Sep 17 00:00:00 2001 From: Haiko Schol Date: Mon, 16 Dec 2024 12:09:24 +0700 Subject: [PATCH 4/5] add mkdocs header --- docs/docs/design/bitfield-signing.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/docs/design/bitfield-signing.md b/docs/docs/design/bitfield-signing.md index c539dcc632..d7930d54a6 100644 --- a/docs/docs/design/bitfield-signing.md +++ b/docs/docs/design/bitfield-signing.md @@ -1,3 +1,9 @@ +--- +layout: default +title: Bitfield Signing Subsystem +permalink: /design/bitfield-signing/ +--- + # Bitfield Signing Subsystem [The bitfield signing subsystem](https://paritytech.github.io/polkadot-sdk/book/node/availability/bitfield-signing.html) From 5ec9c398b4c9538083ee68236b5ae537fe4e50d7 Mon Sep 17 00:00:00 2001 From: Haiko Schol Date: Mon, 16 Dec 2024 17:05:10 +0700 Subject: [PATCH 5/5] address review comments --- docs/docs/design/bitfield-signing.md | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/docs/docs/design/bitfield-signing.md b/docs/docs/design/bitfield-signing.md index d7930d54a6..05ee6ea72c 100644 --- a/docs/docs/design/bitfield-signing.md +++ b/docs/docs/design/bitfield-signing.md @@ -41,7 +41,9 @@ use it in `DistributeBitfield`. 2. [`AvailabilityStore::QueryChunkAvailability(CandidateHash, ValidatorIndex, response_channel)`](https://github.com/paritytech/polkadot-sdk/blob/1e3b8e1639c1cf784eabf0a9afcab1f3987e0ca4/polkadot/node/subsystem-types/src/messages.rs#L556) -This type is already defined as `availabilitystore.QueryChunkAvailability` in +This message is sent once for each occupied core, whenever the subsystem is notified of a new active leaf. + +The type is already defined as `availabilitystore.QueryChunkAvailability` in `dot/parachain/availability-store/messages.go`. ## Subsystem State @@ -72,6 +74,10 @@ type BitfieldSigning struct { } ``` +Ideally the implementation should avoid lock contention around the keystore. Since the signing key remains the same for +the duration of the signing task, the subsystem could pass in the key pair or just the private key. The implementer +should double-check that this approach is thread-safe. + Again, this should probably use a (to be created) type `CheckedSignedAvailabilityBitfield`. ## Message Handling Logic @@ -91,6 +97,4 @@ for the given leaf from the runtime. 3. For each core, concurrently check whether the core is occupied and if so, query the availability store using `QueryChunkAvailability`. -4. Collect the results of the queries into a bitfield, sign it and send it to the subsystem. - -5. In the subsystem, receive the signed bitfield and send a `DistributeBitfield` message to the overseer. +4. Collect the results of the queries into a bitfield, sign it and send a `DistributeBitfield` message to the overseer.