[feature] #4285: Verifiable Random Function in Sumeragi

[SamHSmith]

Description

Adds a crypto primitive and randomization of the topology on block commits.

Linked issue

Closes #4285

Benefits

Checklist

• I've read CONTRIBUTING.md
• I've used the standard signed-off commit format (or will squash just before merging)
• All applicable CI checks pass (or I promised to make them pass later)
• (optional) I've written unit tests for the code changes
• I replied to all comments after code review, marking all implemented changes with thumbs up

[mversic]

is it really a necessity that we store commit_topology in the block? Can't we just use the current topology of sumeragi? Because this field can blow the chain size up significantly

[SamHSmith]

It is. We could imagine some scheme where we store only a delta and can infer the current topology. But I think it's probably fine since the block headers don't stay in memory. Once 2 blocks have passed this data should only be on disk. I say the chance of bugs or mistakes is a lot higher if we try to be smart here. Does not seem worth it.

[mversic]

But I think it's probably fine since the block headers don't stay in memory

that's true

We could imagine some scheme where we store only a delta and can infer the current topology

can't we replay topology while doing block replay? I'm saying it will be computed anyhow, so why store it in the block?

I say the chance of bugs or mistakes is a lot higher if we try to be smart here. Does not seem worth it

I need a better argument why it's necessary to store it in the block header

[SamHSmith]

It isn't necessary, you are right that we could replay the topology with the block replay. It didn't seem worth it to me since the topology is quite small. But if you think we should replay it then I could work on that.

[SamHSmith]

@mversic I misspoke in our call. We would need to sort after we make this change and keep track of Peers without storing it in the block header. Right now we use the commit topology and that will be the same for all peers since it is on the block header. Once we remove the topology from the header we would need track the peers with a UniqueVec for consistent shuffling on different peers. But as of now there is no thing I needed to force push to the PR.

[Erigara]

We still rely on order of signatures, which was highlighted as undesirable in the original issue.

[SamHSmith]

We don't rely on the order of the signatures, we only use them to form the A set.

[Erigara]

I'm not sure that i fully understand the purpose of this change.

Since VRFState looks predetermined from the beginning, so peer that submit genesis could manipulate it.

Because it's computed as signature(signature(rand, genesis), leader).

[Erigara]

Can you provide any sources you used while implementing this approach?

[SamHSmith]

I'm not sure that i fully understand the purpose of this change.

Since VRFState looks predetermined from the beginning, so peer that submit genesis could manipulate it.

Because it's computed as signature(signature(rand, genesis), leader).

signature(rand, genesis) is in practice random. Because if could be predicted then you
could fake signatures. This is the basic verifiable random property. A random number is generated
that only the leader can generate. But everyone else can verify the number so the leader cannot tamper
with it. This means that the topology evolution cannot be tampered with, but only the leader can know
what the next topology will be. This reduces the time for an attacker to coordinate a denial of service attack.

The topology for the next round is known only when a peer receives the block candidate. And nobody is able
to predict the topology two blocks ahead. (Unless by random chance the same leader was picked twice.)

[SamHSmith]

Also the seed value that the genesis peers submits is completely unimportant. It just needs to be some value. I've thought about @mversic 's suggestion that we should replay topology from the start and not store it on each block. I think it is a good idea and I will make the change in this PR.

[Erigara]

Afaik for the single pair of payload and private key there is more than one valid signature.
For elliptic curve signatures this set is large due to selection of pseudorandom number.
So i have a concern that this could be abused by leader to pick itself as leader again.
If we consider that signature is in practice random as you said leader will have to try about n (n is total number of peers) different signatures to find the one that allow him to be the leader for the next round.

EDIT: there is some signature schemas like BLS which has unique signature property.

[SamHSmith]

Good point. I will have to investigate that. It might be that I can encrypt the hash with the private key, that way creating a signing scheme that only has one correct answer. This is the behavior I assumed from our signature primitives. Good that you spotted this.

[mversic]

Also the seed value that the genesis peers submits is completely unimportant. It just needs to be some value. I've thought about @mversic 's suggestion that we should replay topology from the start and not store it on each block. I think it is a good idea and I will make the change in this PR.

@SamHSmith don't remove commit_topology in the scope of this PR

[coveralls]

Pull Request Test Coverage Report for Build 8622562946

Details

• 159 of 185 (85.95%) changed or added relevant lines in 11 files are covered.
• 5584 unchanged lines in 96 files lost coverage.
• Overall coverage increased (+0.9%) to 57.697%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
core/src/block.rs	33	34	97.06%
config/src/parameters/user.rs	8	10	80.0%
data_model/src/isi.rs	0	2	0.0%
core/test_network/src/lib.rs	0	3	0.0%
core/src/sumeragi/main_loop.rs	18	36	50.0%

Files with Coverage Reduction	New Missed Lines	%
primitives/src/conststr.rs	1	91.14%
crypto/src/hash.rs	1	73.78%
ffi/derive/src/convert.rs	1	84.45%
primitives/src/lib.rs	1	0.0%
core/src/sumeragi/network_topology.rs	1	98.78%
primitives/src/must_use.rs	2	74.29%
crypto/src/signature/bls/mod.rs	2	0.0%
config/src/snapshot.rs	3	78.57%
data_model/derive/src/has_origin.rs	3	95.16%
config/src/kura.rs	3	80.0%

Totals
Change from base Build 7884695009:	0.9%
Covered Lines:	23574
Relevant Lines:	40858

---

💛 - Coveralls

[SamHSmith]

Something is wrong with the CI here.