-
Notifications
You must be signed in to change notification settings - Fork 59
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Enforce one transaction per notification in the networking (#56)
[Rendered](https://github.com/tomaka/RFCs-1/blob/one-tx-per-notif/text/0056-one-transaction-per-notification.md) This RFC is pretty small, and I expect it to be pretty uncontroversial. Just because something is currently designed a certain way doesn't mean that it is so for a good reason, and sometimes, like in this RFC, things just need to be cleaned up.
- Loading branch information
Showing
1 changed file
with
96 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,96 @@ | ||
# RFC-0056: Enforce only one transaction per notification | ||
|
||
| | | | ||
| --------------- | ------------------------------------------------------------------------------------------- | | ||
| **Start Date** | 2023-11-30 | | ||
| **Description** | Modify the transactions notifications protocol to always send only one transaction at a time| | ||
| **Authors** | Pierre Krieger | | ||
|
||
## Summary | ||
|
||
When two peers connect to each other, they open (amongst other things) a so-called "notifications protocol" substream dedicated to gossiping transactions to each other. | ||
|
||
Each notification on this substream currently consists in a SCALE-encoded `Vec<Transaction>` where `Transaction` is defined in the runtime. | ||
|
||
This RFC proposes to modify the format of the notification to become `(Compact(1), Transaction)`. This maintains backwards compatibility, as this new format decodes as a `Vec` of length equal to 1. | ||
|
||
## Motivation | ||
|
||
There exists three motivations behind this change: | ||
|
||
- It is technically impossible to decode a SCALE-encoded `Vec<Transaction>` into a list of SCALE-encoded transactions without knowing how to decode a `Transaction`. That's because a `Vec<Transaction>` consists in several `Transaction`s one after the other in memory, without any delimiter that indicates the end of a transaction and the start of the next. Unfortunately, the format of a `Transaction` is runtime-specific. This means that the code that receives notifications is necessarily tied to a specific runtime, and it is not possible to write runtime-agnostic code. | ||
|
||
- Notifications protocols are already designed to be optimized to send many items. Currently, when it comes to transactions, each item is a `Vec<Transaction>` that consists in multiple sub-items of type `Transaction`. This two-steps hierarchy is completely unnecessary, and was originally written at a time when the networking protocol of Substrate didn't have proper multiplexing. | ||
|
||
- It makes the implementation way more straight-forward by not having to repeat code related to back-pressure. See explanations below. | ||
|
||
## Stakeholders | ||
|
||
Low-level developers. | ||
|
||
## Explanation | ||
|
||
To give an example, if you send one notification with three transactions, the bytes that are sent on the wire are: | ||
|
||
``` | ||
concat( | ||
leb128(total-size-in-bytes-of-the-rest), | ||
scale(compact(3)), scale(transaction1), scale(transaction2), scale(transaction3) | ||
) | ||
``` | ||
|
||
But you can also send three notifications of one transaction each, in which case it is: | ||
|
||
``` | ||
concat( | ||
leb128(size(scale(transaction1)) + 1), scale(compact(1)), scale(transaction1), | ||
leb128(size(scale(transaction2)) + 1), scale(compact(1)), scale(transaction2), | ||
leb128(size(scale(transaction3)) + 1), scale(compact(1)), scale(transaction3) | ||
) | ||
``` | ||
|
||
Right now the sender can choose which of the two encoding to use. This RFC proposes to make the second encoding mandatory. | ||
|
||
The format of the notification would become a SCALE-encoded `(Compact(1), Transaction)`. | ||
A SCALE-compact encoded `1` is one byte of value `4`. In other words, the format of the notification would become `concat(&[4], scale_encoded_transaction)`. | ||
This is equivalent to forcing the `Vec<Transaction>` to always have a length of 1, and I expect the Substrate implementation to simply modify the sending side to add a `for` loop that sends one notification per item in the `Vec`. | ||
|
||
As explained in the motivation section, this allows extracting `scale(transaction)` items without having to know how to decode them. | ||
|
||
By "flattening" the two-steps hierarchy, an implementation only needs to back-pressure individual notifications rather than back-pressure notifications and transactions within notifications. | ||
|
||
## Drawbacks | ||
|
||
This RFC chooses to maintain backwards compatibility at the cost of introducing a very small wart (the `Compact(1)`). | ||
|
||
An alternative could be to introduce a new version of the transactions notifications protocol that sends one `Transaction` per notification, but this is significantly more complicated to implement and can always be done later in case the `Compact(1)` is bothersome. | ||
|
||
## Testing, Security, and Privacy | ||
|
||
Irrelevant. | ||
|
||
## Performance, Ergonomics, and Compatibility | ||
|
||
### Performance | ||
|
||
Irrelevant. | ||
|
||
### Ergonomics | ||
|
||
Irrelevant. | ||
|
||
### Compatibility | ||
|
||
The change is backwards compatible if done in two steps: modify the sender to always send one transaction per notification, then, after a while, modify the receiver to enforce the new format. | ||
|
||
## Prior Art and References | ||
|
||
Irrelevant. | ||
|
||
## Unresolved Questions | ||
|
||
None. | ||
|
||
## Future Directions and Related Material | ||
|
||
None. This is a simple isolated change. |