Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fix] #2133: Rewrite topology to be closer the whitepaper. #3250

Conversation

SamHSmith
Copy link
Contributor

Description of the Change

Issue

Benefits

Possible Drawbacks

Usage Examples or Tests [optional]

Alternate Designs [optional]

@SamHSmith SamHSmith self-assigned this Feb 22, 2023
@github-actions github-actions bot added the iroha2-dev The re-implementation of a BFT hyperledger in RUST label Feb 22, 2023
@SamHSmith SamHSmith marked this pull request as draft February 22, 2023 22:51
@SamHSmith SamHSmith marked this pull request as ready for review February 22, 2023 22:52
@codecov
Copy link

codecov bot commented Feb 22, 2023

Codecov Report

Merging #3250 (21244c4) into iroha2-dev (a4d5c9f) will increase coverage by 0.34%.
The diff coverage is 54.98%.

❗ Current head 21244c4 differs from pull request most recent head b563ae1. Consider uploading reports for the commit b563ae1 to get more accurate results

@@              Coverage Diff               @@
##           iroha2-dev    #3250      +/-   ##
==============================================
+ Coverage       62.33%   62.67%   +0.34%     
==============================================
  Files             169      159      -10     
  Lines           31218    31826     +608     
==============================================
+ Hits            19459    19948     +489     
- Misses          11759    11878     +119     
Impacted Files Coverage Δ
cli/src/main.rs 0.85% <0.00%> (-0.25%) ⬇️
cli/src/samples.rs 57.97% <ø> (-3.88%) ⬇️
cli/src/torii/mod.rs 27.65% <ø> (ø)
client_cli/src/main.rs 0.24% <0.00%> (-0.01%) ⬇️
config/base/src/lib.rs 91.57% <ø> (+55.21%) ⬆️
config/src/lib.rs 33.33% <ø> (ø)
config/src/path.rs 0.00% <0.00%> (ø)
config/src/wasm.rs 100.00% <ø> (ø)
core/src/lib.rs 100.00% <ø> (ø)
core/src/smartcontracts/isi/asset.rs 50.88% <0.00%> (-2.65%) ⬇️
... and 185 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

crypto/src/signature.rs Outdated Show resolved Hide resolved
core/src/sumeragi/network_topology.rs Outdated Show resolved Hide resolved
core/src/sumeragi/network_topology.rs Outdated Show resolved Hide resolved
@Erigara
Copy link
Contributor

Erigara commented Feb 27, 2023

@SamHSmith can you please explain why we need to carry topology in every block since every peer anyway uses it's own topology to verify that received topology is correct?

core/src/sumeragi/network_topology.rs Show resolved Hide resolved
core/src/sumeragi/network_topology.rs Outdated Show resolved Hide resolved
core/src/sumeragi/network_topology.rs Outdated Show resolved Hide resolved
core/src/sumeragi/network_topology.rs Show resolved Hide resolved
core/src/sumeragi/network_topology.rs Outdated Show resolved Hide resolved
@SamHSmith
Copy link
Contributor Author

@SamHSmith can you please explain why we need to carry topology in every block since every peer anyway uses it's own topology to verify that received topology is correct?

The old topology was based on a block has and a view change index. This is bad for security since the leader can influence the block hash and therefore influence the future topology.

Now that we have a constantly evolving topology whos operations are destructive, rolling back becomes harder. Instead of keeping old topologies around locally we store it in the block header. This way when there is a softfork or odd consensus a peer can see that enough peers signed this block and adopt the topology. Topology is checked while verifying but not when receiving block through syncing.

@SamHSmith SamHSmith force-pushed the revise_view_change_method branch 5 times, most recently from a38a85d to 8d91fd1 Compare March 6, 2023 21:20
@Erigara
Copy link
Contributor

Erigara commented Mar 7, 2023

The old topology was based on a block has and a view change index. This is bad for security since the leader can influence the block hash and therefore influence the future topology.

So now topology is kinda predetermined, right?

@SamHSmith
Copy link
Contributor Author

The old topology was based on a block has and a view change index. This is bad for security since the leader can influence the block hash and therefore influence the future topology.

So now topology is kinda predetermined, right?

Kinda is the right word to use. Because it's not exactly predetermined in the sense that you could predict it. But it is a result of what happens in a given round, block signing etc. So you cannot predict it but all peers will agree on the topology if they observe the same events taking place.

@SamHSmith SamHSmith force-pushed the revise_view_change_method branch 2 times, most recently from 075f418 to 9e60a12 Compare March 8, 2023 12:32
appetrosyan
appetrosyan previously approved these changes Mar 9, 2023
@SamHSmith SamHSmith force-pushed the revise_view_change_method branch 2 times, most recently from 0e3b14f to 7f90b83 Compare March 9, 2023 09:58
appetrosyan
appetrosyan previously approved these changes Mar 9, 2023
core/src/genesis.rs Outdated Show resolved Hide resolved
@SamHSmith SamHSmith force-pushed the revise_view_change_method branch 2 times, most recently from 7b9d598 to 21244c4 Compare March 9, 2023 14:47
core/src/genesis.rs Outdated Show resolved Hide resolved
…paper.

Signed-off-by: Sam H. Smith <sam.henning.smith@protonmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
iroha2-dev The re-implementation of a BFT hyperledger in RUST
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants