ModifyBlock layout (replace/add fields)

[stanislav-tkach]

Do we still need it?

[dj8yfo]

@slowli why remove this info? it's the only persistent field to track the actual proposer of block, i believe (beside logs). It could be used to track behavior of individual validators, censorship etc.

[dj8yfo]

better off with #76 instead, if we're ok with modifying block a little prior to 0.1.0, so as to commit to block length and so that inclusion proofs become "more" legit.
@defuz that's the approach i believe you stated you would adhere to. Instead of hash prefixing stuff.

[slowli]

IMO, tracking proposal censorship (or failing validators) is a viable use case, and if we can do this automatically (say, using statistical tests - chi-square, KS, or other one - via a dedicated service), it would be a good thing, I believe. Does removing the propose_round field harm this use case?

On the other hand, propose_round is most probably insufficient for non-repudiation of the block contents; the Propose message of the block author (including the digital signature) is necessary for that. I'm not sure whether digital signatures of "successful" Propose messages contribute to the blockchain state (which could be timestamped by anchoring and which maintains integrity via hash links); if not, it could harm non-repudiation as well. Storing these proposals, however, could probably be implemented as a service.

[stanislav-tkach]

I don't like propose_round because it is used in a strange indirect way and we have to duplicate leader logic in several places. As for me, if we need to know block leader, then we should store leader id or public key directly.

[stanislav-tkach]

One more thing: if nodes have been added (or removed) in process, then we get wrong results using propose_round, don't we?

[dj8yfo]

No, we don't (added, removed).
We would calculate proposer_id = (propose_round + block_height)%validators_len_at_height.

Where validators_len_at_height would be configuration_by_height(block_height).validators.len()

[dj8yfo]

@DarkEld3r @slowli
Because Propose message is itself quite big (it lists all tx hashes, so potentially could be like 96kb size with 3000 txs per block parameter).
i see 3 options (exclusive), how we could proceed with this task. To verify Propose signature the hashes list could be recovered from already stored schema.block_txs(block_height).values().

1. simply replace propose_round with proposer_id to remove some indirection. It would be better if it were proposer_public_key, but we haven't changed that in all other places. So .._id for consistency. For now.

2. add following fields to Block from Propose message:

   field validator:      u32         [00 => 04]  //aka proposer_id
   

   The following are already present:

   field height:         u64         [04 => 12]
   field round:          u32         [12 => 16]
   field prev_hash:      &Hash       [16 => 48]
   

   Add signature of Propose message.
   Then the full Propose message could be reconstructed and verified, if needed. And confirmed , that it originated from named validator.

   field transactions: &[Hash] [48 => 56] in Propose < - > field tx_hash: &Hash [44 => 76] in Block.
   The latter is merkle root of the former.

3. Do not add additional fields to Block. Replace propose_round with propose_message_hash. Store the missing fields and signature of Propose in some other table.

[dj8yfo]

@defuz What do you think?
I think option 1. could be done with small effort now. If do anything at all for this task now.

[dj8yfo]

4. decompose Propose into 2 messages - a compact one with tx_hash merkle root and a longer one with actual listing of tx hashes, referencing the first one. This option would require significant changes to consensus algorithm.

[dj8yfo]

@DarkEld3r @deniskolodin @aleksuss we've decided on following set of changes prior to 0.1.0:

1. replace propose_round with proposer_id - u32. proposer_id should be equal to validator of corresponding Propose.

2. add schema_version - u16. And populate it statically with 0 everywhere. For now.
   This is like protocol_version in messages, but these values do not directly influence each other - protocol_version can change without affecting schema_version and vice versa.

3. add network_id - u8. Populate it statically with 0 everywhere. This one is assumed to be the same as network_id in messages.

Please, proceed with above spec.

[therustmonk]

@gisochre These are new fields for Block, are they?
Do I need to put schema_version at the beginning of the block?
This order helps us to detect the schema version before reading all data from the block.

[dj8yfo]

yep, at the beginnning. schema_version - u16, sorry.
I believe we currently read the whole message anyway. When receiving over network. And when reading from db.

[dj8yfo]

@deniskolodin you'll be shocked, but:

1. remove network_id from Block
2. change proposer_id from u32 to u16. And in Propose from u32 to u16 too. And all validator_id from u32 to u16 everywhere.
3. replace tx_count with u32 in Block. (and maybe somewhere else). But seems to be nowhere else 🥇. You'll have to cast from u64 when block_txs(&self, height: u64). Or not).

Motivation:
to align field prev_hash: &Hash [23 => 55] as 16 and not 23. And we don't need network_id in each block(probably only in genesis).
And proposer_id is unrealistic to be over 2^16. And tx_count over 2^32 per block is slightly too much.

[therustmonk]

Small stuff ) I'll fix it.

[therustmonk]

I've implemented all with #139 except:

decompose Propose into 2 messages - a compact one with tx_hash merkle root and a longer one with actual listing of tx hashes, referencing the first one. This option would require significant changes to consensus algorithm.

Is the last relevant?