Initial erasure-coding of availability data #56

rphmeier · 2018-12-14T17:07:05Z

related: #51
This is most of step 2 in that issue -- the remaining bit is that the merkle root should be added to the candidate receipt.

This introduces the polkadot-erasure-coding crate, which will be used to create erasure-codings of all parachain data which much be kept available. With n validators, f maximum faulty, we create a coding of n pieces where any f+1 can be used to recover the data.

The crate for now contains utilities for:

creating and reconstructing erasure-coded data
merkleizing the chunks into a mapping of row_number -> hash(row_data)
checking merkle branches

Currently this has a limit on the number of supported validators at 65536, due to the fact that the underlying reed-solomon-erasure crate only supports GF(2^16). This limit is unlikely to prove to be an issue for some time.

rphmeier · 2019-01-10T16:22:01Z

erasure-coding/Cargo.toml

+
+[dependencies]
+polkadot-primitives = { path = "../primitives" }
+reed-solomon-erasure = { git = "https://github.com/paritytech/reed-solomon-erasure", branch = "rh-usable-gf16" }


We will switch to "0.4" when that is published.

rphmeier · 2019-01-15T00:14:39Z

I don't know why it says the travis build failed; if you go to the site it says it passed.

gavofyork · 2019-01-18T08:53:11Z

would be good to get a review from someone familiar with the underlying algorithm (like jeff?). code looks clean enough from my pov.

rphmeier · 2019-01-24T15:31:10Z

(erasure-coding logic was reviewed by @drskalman outside of github)

burdges · 2019-02-19T10:44:43Z

erasure-coding/src/lib.rs

+impl CodeParams {
+	// the shard length needed for a payload with initial size `base_len`.
+	fn shard_len(&self, base_len: usize) -> usize {
+		(base_len / self.data_shards) + (base_len % self.data_shards)


I'd expect (base_len-1) / self.data_shards + 1 here.

After the confusion below I'd expect (base_len-1+8) / self.data_shards + 1 here. And modify the last shard to state its length at the end. And @drskalman points out that parity codec just ads the length too.

8 seems very arbitrary?

base_len / self.data_shards + (base_len % self.data_shards != 0) as usize

burdges · 2019-02-19T11:05:16Z

erasure-coding/src/lib.rs

+		(base_len / self.data_shards) + (base_len % self.data_shards)
+	}
+
+	fn make_shards_for(&self, payload: &[u8]) -> Vec<WrappedShard> {


According to @drskalman parity codec handled the length so I'd expect very roughly:

pub(crate) struct WrappedShard<B: Borrow<[u8]>> { inner: B, } fn make_shards_for<'a>(&self, payload: &'a [u8]) -> impl Iterator<Item=WrappedShard<Cow<'a,[u8]>>> { let shard_len = self.shard_len(payload.len()); payload.chunks(shard_len).map(|c| { if c.len() == shard_len { WrappedShard { inner: Cow::Borrowed(c) } } else { let mut moo = vec![0; shard_len]; let l = c.len(); moo[..l].copy_from_slice(&c[..l]); WrappedShard { inner: Cow::Owned(moo) } } }) }

but I suppose Cows are complete overkill here, so probably just Vec and the else branch for all.

I suppose the existing code works fine actually. I just expected to need to change this fn after shortening shard_len above.

Oh I see! You need the total payload length encoded. Sorry I didn't read very carefully.

In any case, your current shard_len function will give a bunch of zero len shards, but the current code works I think.

So I suggest to use payload.encode() instead of payload and then don't worry about the length as all shards are the same size and we recover the total length using SCALE codec.

The reason we prepend the length to each shard is explained in the comment above. It's because lists of element of GF(2^16), when treated as byte-slices, always have even length. However, our shard length might be odd. So we need to signal whether there is a trailing zero byte that we should skip when decoding, and prepending the length to each shard has been an easy way to do that (although not the most efficient). I filed #88 to address that.

I don't think prepending the length to the payload actually helps at all.

What I will do, I think, is round up shard_len to the next even number. Then, the last shards will have (cumulatively) n_validators more zero bytes, but for long payloads this doesn't really matter much. And it's easier to decode anyway, since we don't have to reason about the extra byte at the end.

burdges · 2019-02-19T11:22:21Z

erasure-coding/src/lib.rs

+		return Err(Error::BadPayload);
+	}
+
+	let mut shards = params.make_shards_for(&encoded[..]);


Ahh okay no reason in Cows here, even if the ones outside my house are cute. ;) And no reason for the complexity of arenas here.

burdges · 2019-02-19T11:24:04Z

erasure-coding/src/lib.rs

+	// make a reed-solomon instance.
+	fn make_encoder(&self) -> ReedSolomon {
+		ReedSolomon::new(self.data_shards, self.parity_shards)
+			.expect("this struct is not created with invalid shard number; qed")


double negative in this expect string

burdges · 2019-02-19T11:29:27Z

erasure-coding/src/lib.rs

+	-> Result<(BlockData, Extrinsic), Error>
+	where I: IntoIterator<Item=(&'a [u8], usize)>
+{
+	let params = code_params(n_validators)?;


We might worry about n_validators being the same on both sides here. We also cannot trust the encoded data for n_validators because then an adversary can manipulate it, so I believe this is fine but maybe the comment should emphasize that n_validators must be correct or else we create invalid slashing attacks.

Also I think you meant more than 256 here

I think the generator of the erasure code should append n to each shard along side with the Merkle root and sign on it. I'll talk to Al to update the protocol.

Al says the number of Validators changes in order of once a year. And it is off course retrievable from state root. He also reiterate there is no harm to add this the pieces. Your call then.

burdges · 2019-02-19T12:28:28Z

erasure-coding/src/lib.rs

+	where I: IntoIterator<Item=(&'a [u8], usize)>
+{
+	let params = code_params(n_validators)?;
+	let mut shards: Vec<Option<WrappedShard>> = vec![None; n_validators];


You could maybe avoid the option by doing map and collect below, but whatever.

I haven't looked at this code for a while, but I think the Option is necessary because of the API of erasure-coding.

burdges · 2019-02-19T12:31:54Z

erasure-coding/src/lib.rs

+	let params = code_params(n_validators)?;
+	let mut shards: Vec<Option<WrappedShard>> = vec![None; n_validators];
+	let mut shard_len = None;
+	for (chunk_data, chunk_idx) in chunks.into_iter().take(n_validators) {


I'd probably do

let mut chunks = chunks.into_iter(); for (chunk_data, chunk_idx) in chunks.by_ref().take(n_validators) { ... } if chunks.next() != None { return Err(...); }

burdges · 2019-02-19T12:39:16Z

Just curious, I take it both Branches and fns like reconstruct get called from another crate, so presumably reconstruct does not quite work as a method on Branches or whatever? Not that this matters much. :)

burdges · 2019-02-19T12:46:35Z

I'm happy with this modulo wasting 4 bytes per shard and CodeParams::shard_len increasing the size by more than the required 8 bytes, or 4 bytes per shard with the current code. Also, some comments could be improved, especially any warnings around n_validators being wrong. I have not looked at https://github.com/paritytech/reed-solomon-erasure as much as I'd like.

drskalman · 2019-01-25T17:07:09Z

erasure-coding/src/lib.rs

+impl CodeParams {
+	// the shard length needed for a payload with initial size `base_len`.
+	fn shard_len(&self, base_len: usize) -> usize {
+		(base_len / self.data_shards) + (base_len % self.data_shards)


base_len=19 data_shards = 10 logically shard of length 2 should do but now we end up with 19/10 + 9= 10 shard of length 10? So what @burdges says: (base_len-1) / self.data_shards + 1

I don't think that's right, though.

base_len = 19 data_shards = 5. We need shard length 4 to fit the whole payload.

18 / 6 = 3.

the formula is

((base_len -1)/ self.data_shards) + 1

(divison has higher priority than addition) or simpler

(base_len + self.data_shards - 1)/self.data_shards

but as you said we should avoid odd shard length so check if it is odd and add one to it.

drskalman · 2019-02-19T13:29:56Z

erasure-coding/src/lib.rs

+		(base_len / self.data_shards) + (base_len % self.data_shards)
+	}
+
+	fn make_shards_for(&self, payload: &[u8]) -> Vec<WrappedShard> {


So I suggest to use payload.encode() instead of payload and then don't worry about the length as all shards are the same size and we recover the total length using SCALE codec.

drskalman · 2019-02-19T13:30:50Z

erasure-coding/src/lib.rs

+/// The indices of the present chunks must be indicated. If too few chunks
+/// are provided, recovery is not possible.
+///
+/// Works only up to 256 validators, and `n_validators` must be non-zero.


So this needs to be 65536

drskalman · 2019-02-19T13:33:39Z

erasure-coding/src/lib.rs

+	-> Result<(BlockData, Extrinsic), Error>
+	where I: IntoIterator<Item=(&'a [u8], usize)>
+{
+	let params = code_params(n_validators)?;


I think the generator of the erasure code should append n to each shard along side with the Merkle root and sign on it. I'll talk to Al to update the protocol.

drskalman · 2019-02-19T13:40:03Z

erasure-coding/src/lib.rs

+
+/// Obtain erasure-coded chunks, one for each validator.
+///
+/// Works only up to 256 validators, and `n_validators` must be non-zero.


The bound should be 65536

drskalman · 2019-02-19T14:05:35Z

erasure-coding/src/lib.rs

+	-> Result<(BlockData, Extrinsic), Error>
+	where I: IntoIterator<Item=(&'a [u8], usize)>
+{
+	let params = code_params(n_validators)?;


Al says the number of Validators changes in order of once a year. And it is off course retrievable from state root. He also reiterate there is no harm to add this the pieces. Your call then.

drskalman · 2019-02-19T14:05:49Z

erasure-coding/src/lib.rs

+
+		assert_eq!(proofs.len(), 10);
+
+		for (i, proof) in proofs.into_iter().enumerate() {


I'm not sure, I'm getting this correctly. I think each Merkle proof should contains the hash value of the siblings of all nodes in your branch. So you can compute your hash way up to the root. Am I missing something?

we are just including all the nodes on the path -- that's how you generally do merkle proofs for 16-radix (which we are using here)

I verified with @burdges, you need the co-path not the path. On a binary tree it is all the other children of the nodes on the path.

@burdges why do we need the co-path? the path seems to work (although can be optimized by omitting the hash of the child we are traversing to from branches)

(have chatted with @burdges in Riot and confirmed that it is correct as-is for the patricia trie)

burdges · 2019-02-21T21:52:43Z

In a Merkle tree, we always use siblings of the actual path, which I've once or twice heard referred to as a copath. I have not yet understood at what point TrieDB, etc. actually hashes the siblings inside https://github.com/paritytech/trie/blob/master/trie-db/src/triedbmut.rs#L850 but maybe it's buried inside this DB crate, except they looked like just hash maps.

Just to clarify terminology:

If I have a Merkle tree with four elements then I compute the root like

root = H(H(x[0] ++ x[1]), H(x[2] ++ x[3]))

so a proof of inclusion for x[1] has the form p = [ Right(x[0]), Left(H(x[2] ++ x[3])) ] so the proof can be checked by testing root == H(H(p[0], x[1]), p[1]).

In usual terminology, the "branch" containing x[1], or equivalently the "path" from x[1] to the root, would instead be b = [ x[1], H(x[0] ++ x[1]), root ].

Both sequences do end with the root, but the branch provides no inclusion proof. All values in the branch are computed in verifying the inclusion proof, but the two important values in the inclusion proof cannot be computed from the branch.

I presume this module should produce the inclusion proofs somewhere?

burdges · 2019-02-21T22:16:44Z

Rob explained that each level should contain all 16 children, which explains the extra nested Vec too probably. We could improve proof size by a factor of 4 by hashing as if the patricia trie was a binary tree, but I'm unsure how deep that goes into this crate hierarchy, or if the code is optimized for cases that do not require proofs.

burdges · 2019-02-21T22:54:59Z

I'm satisfied. It's a strange library design heavily optimized for other things, but whatever.

burdges · 2019-02-23T10:08:17Z

At present, we verify a proof by inserting a branch into a patricia trie database using trie::HashDB::insert, which presumably arranges the inserted nodes into children according to their hashes. It's important to provide the nodes in order so the trie gets built correctly, but one could likely provide more than one branch. We might find this convenient if we needed to provide more than one proof together, but risks could emerge from providing multiple proofs together too. Afaik, any attack requires manipulating validator's perception of their own index position, which breaks everything anyways, but it's worth emphasizing the sensitiveness of the index position.

Also, we can compress these proofs by a factor of like 8 by using 16 byte hashes and doing the hashing as a binary tree instead of doing 16 fragments at each depth.

rphmeier · 2019-02-23T15:49:03Z

@burdges

At present, we verify a proof by inserting a branch into a patricia trie database using trie::HashDB::insert, which presumably arranges the inserted nodes into children according to their hashes. It's important to provide the nodes in order so the trie gets built correctly

this is not correct. MemoryDB is just a lookup from hash to node data, and in fact is implemented with a HashMap internally -- whose order is non-deterministic.

The TrieDB struct starts at the root and traverses downwards down the tree by looking up nodes in the HashDB.

Check out the definition of the patricia trie -- I'm not sure that the binary tree hashing method you describe is actually more efficient, because we are currently only doing 1 hash for each node we traverse.

The only optimization I am aware that you can do is a space optimization where you inline the nodes in the path that you are interested in and transmit everything inline.

burdges · 2019-02-23T23:08:35Z

The TrieDB struct starts at the root and traverses downwards down the tree by looking up nodes in the HashDB.

Ok but the point remains: Your adversary can build anything they like here, which appears harmless but sounds less fault tolerant elsewhere.

I'm not sure that the binary tree hashing method you describe is actually more efficient, because we are currently only doing 1 hash for each node we traverse.

A (virtual) binary tree for hashing is 8 times more space efficient. As you consume 8 times less data, it'll have similar performance for creation if you optimize the hash function choice, but verification should be much faster my way. If you look into the blake2b analysis, you might find that chacha alone suffices to hash 2 x 16 bytes into 16 bytes, so creation costs 14 chacha runs vs like 16ish now, but verification cost only like 3 chacha vs the same 16ish the current way. You could reduce to 16 bytes without doing binary tree hashing, but verification remains slower.

rphmeier · 2019-02-23T23:59:12Z

Yeah, the weakness in this code right now is that someone providing a proof could provide a bunch of extra data that is not in the lookup path.

Because we know about the trie structure and the fact that it is a u16 -> [u8; 32], we can set upper bounds on both the number of nodes and number of bytes per node. Anything higher than that can be rejected and any peer serving us that can be disconnected (although we haven't yet done the networking portion of this service).

Fixes paritytech#56 and paritytech#44

rphmeier added the A1-onice label Dec 14, 2018

rphmeier added 4 commits January 9, 2019 17:00

erasure-coding block data

c880ba9

adjust error handling

0a3f3ab

merkleize chunks and yield branches for each

57bf384

construction and proving of merkle branches

e299095

rphmeier force-pushed the rh-erasure-coding branch from f366062 to e299095 Compare January 9, 2019 16:00

rphmeier added 3 commits January 10, 2019 11:22

port over to new GF(2^16) impl

e4305c3

some tests for wrapped_shard

610ecb1

handle extra byte from GF(2^16) better

8a17967

rphmeier added A0-please_review Pull request needs code review. and removed A1-onice labels Jan 10, 2019

point to github dependency

7d423b8

rphmeier commented Jan 10, 2019

View reviewed changes

Merge branch 'master' into rh-erasure-coding

ae71720

rphmeier added 3 commits January 18, 2019 10:17

add issue link

413981e

point to master for reed-solomon-erasure

66e8e87

Merge branch 'master' into rh-erasure-coding

9efa474

rphmeier mentioned this pull request Jan 22, 2019

Optimize erasure-coding marshalling #88

Closed

add missing license header

69e5b91

rphmeier merged commit a976729 into master Jan 24, 2019

burdges reviewed Feb 19, 2019

View reviewed changes

drskalman reviewed Feb 19, 2019

View reviewed changes

rphmeier mentioned this pull request Feb 19, 2019

some erasure-coding tweaks #143

Merged

rphmeier deleted the rh-erasure-coding branch February 23, 2019 15:51

rphmeier mentioned this pull request Feb 24, 2019

availability erasure-coding: limit proof size #157

Closed

imstar15 pushed a commit to imstar15/polkadot that referenced this pull request Aug 25, 2021

Run polkadot using future (paritytech#63)

f8631d8

Fixes paritytech#56 and paritytech#44


		assert_eq!(proofs.len(), 10);

		for (i, proof) in proofs.into_iter().enumerate() {

Initial erasure-coding of availability data #56

Initial erasure-coding of availability data #56

Conversation

rphmeier commented Dec 14, 2018 • edited

Choose a reason for hiding this comment

rphmeier commented Jan 15, 2019

gavofyork commented Jan 18, 2019

rphmeier commented Jan 24, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rphmeier Feb 19, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rphmeier Feb 19, 2019 • edited

Choose a reason for hiding this comment

rphmeier Feb 19, 2019 • edited

Choose a reason for hiding this comment

burdges Feb 19, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

burdges commented Feb 19, 2019

burdges commented Feb 19, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

burdges commented Feb 21, 2019 • edited

burdges commented Feb 21, 2019

burdges commented Feb 21, 2019

burdges commented Feb 23, 2019

rphmeier commented Feb 23, 2019 • edited

burdges commented Feb 23, 2019

rphmeier commented Feb 23, 2019 • edited

rphmeier commented Dec 14, 2018 •

edited

rphmeier Feb 19, 2019 •

edited

rphmeier Feb 19, 2019 •

edited

rphmeier Feb 19, 2019 •

edited

burdges Feb 19, 2019 •

edited

burdges commented Feb 21, 2019 •

edited

rphmeier commented Feb 23, 2019 •

edited

rphmeier commented Feb 23, 2019 •

edited