polkadot-spec
polkadot-spec copied to clipboard
Incoperate recommendations from Gossamer Audit
There have been a lot of good recommendations in the gossamer audit. I will make issues out of them, so that we can update our spec accordingly.
The recommendations:
-
[x] The spec should be explicit regarding how the decoding operation should handle situations where extra data is at the end of some SCALE encoded blob. That is, whether decoding is lenient and returns without an error, or strict and returns an error.
-
[x] The spec, in particular section 2.1 makes no mention of whether the Storage Trie should allow empty values and empty keys, and how they should be handled. In particular Definition 2.4 doesn't clarify whether
storage_set(k, "")andstorage_clear(k)are equivalent operations. The specification appears to imply that empty keys and values are allowed, and both Gossamer and the Substrate reference implementation abide by this, but an explicit definition is preferred. -
[x] Section 6.1.1, Remark 6.2 states that "In Polkadot, all authorities have the weight w_A = 1", but the threshold in Definition 6.12 simply uses the number of authorities instead of the sum of their weights. In this regard, Gossamer follows the spec but Substrate reads the weights from the runtime. The main impact is that this could cause a consensus split should a runtime upgrade set authority weights other than 1.
-
[x] Definition 6.12 is insufficient in clarifying how these non-integer computations should be performed - Doesn't specify floating points or how it should be implemented - so rounding and inconsistent results might cause a consensus split (i.e. when the rounding occurs) - is this floating point? fixed decimal? etc
-
[x] The VRF section A.4 is empty, though this appears to be tracked in https://github.com/w3f/polkadot-spec/issues/467
-
[x] Section 2.1 states: "There is no assumption either on the size of the key nor on the size of the data stored under them, besides the fact that they are byte arrays with specific upper limits on their length. The limit is imposed by the encoding algorithms to store the key and the value in the storage trie". For clarity, consider explicitly specifying the associated length limits or link to a section that does.
@lamafab What is left to do here?
Definition 6.12 is insufficient in clarifying how these non-integer computations should be performed - Doesn't specify floating points or how it should be implemented - so rounding and inconsistent results might cause a consensus split (i.e. when the rounding occurs) - is this floating point? fixed decimal? etc
"Hot-fix" in #529