Skip to content
New issue

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

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

Already on GitHub? # to your account

[fix] #4140: Fix registration of new peer #4142

Merged
merged 2 commits into from
Dec 14, 2023

Conversation

mversic
Copy link
Contributor

@mversic mversic commented Dec 12, 2023

Description

  • Fixed new peer registration
  • Fixed peer restart

Linked issue

Closes #4140

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

@github-actions github-actions bot added the iroha2-dev The re-implementation of a BFT hyperledger in RUST label Dec 12, 2023
@mversic mversic force-pushed the fix_peer_registration branch from cb60164 to 2500149 Compare December 12, 2023 16:46
@coveralls
Copy link

coveralls commented Dec 12, 2023

Pull Request Test Coverage Report for Build 7193927722

  • 51 of 65 (78.46%) changed or added relevant lines in 10 files are covered.
  • 7105 unchanged lines in 125 files lost coverage.
  • Overall coverage decreased (-3.3%) to 56.119%

Changes Missing Coverage Covered Lines Changed/Added Lines %
core/src/block.rs 17 18 94.44%
core/src/sumeragi/mod.rs 0 1 0.0%
core/test_network/src/lib.rs 0 1 0.0%
p2p/src/network.rs 0 1 0.0%
smart_contract/executor/derive/src/validate.rs 3 4 75.0%
core/src/sumeragi/main_loop.rs 2 11 18.18%
Files with Coverage Reduction New Missed Lines %
config/base/derive/src/view.rs 1 99.37%
core/src/smartcontracts/isi/block.rs 1 87.5%
ffi/src/option.rs 2 71.43%
config/src/genesis.rs 3 72.92%
config/src/logger.rs 3 92.68%
data_model/derive/src/has_origin.rs 3 95.16%
data_model/src/events/execute_trigger.rs 3 30.77%
data_model/src/peer.rs 3 47.62%
ffi/src/ir.rs 3 80.0%
ffi/src/lib.rs 3 93.46%
Totals Coverage Status
Change from base Build 5423219773: -3.3%
Covered Lines: 22783
Relevant Lines: 40598

💛 - Coveralls

@mversic mversic force-pushed the fix_peer_registration branch 3 times, most recently from f93c3d5 to 06bd388 Compare December 12, 2023 18:08
@mversic mversic self-assigned this Dec 13, 2023
@mversic mversic force-pushed the fix_peer_registration branch 9 times, most recently from ef697c4 to cd810b7 Compare December 13, 2023 08:50
@Erigara Erigara self-assigned this Dec 13, 2023
client/tests/integration/connected_peers.rs Outdated Show resolved Hide resolved
config/src/sumeragi.rs Show resolved Hide resolved
docker-compose.single.yml Outdated Show resolved Hide resolved
@Erigara
Copy link
Contributor

Erigara commented Dec 13, 2023

I wounder how we should deal with more general case when whole topology changed.

Step by step:

  1. Network is started with peer set A
  2. After some height peer set A is replaced with peer set B (A intersection B = empty set)
  3. New peer is added with trusted peers peers from set B
  4. Peer get block and update it's topology to set A (because at this height only peers from topology A were present)

@mversic
Copy link
Contributor Author

mversic commented Dec 13, 2023

I wounder how we should deal with more general case when whole topology changed.

good point. We should discuss the best solution in a separate ticket. Created #4145

@mversic mversic force-pushed the fix_peer_registration branch 3 times, most recently from c5fa890 to c103829 Compare December 13, 2023 10:19
@mversic mversic requested a review from Erigara December 13, 2023 10:30
@mversic mversic force-pushed the fix_peer_registration branch from c103829 to 2def11f Compare December 13, 2023 10:30
Erigara
Erigara previously approved these changes Dec 13, 2023
@mversic mversic force-pushed the fix_peer_registration branch 4 times, most recently from 74ed882 to 8c10072 Compare December 13, 2023 16:05
@mversic mversic force-pushed the fix_peer_registration branch 3 times, most recently from b881c14 to 87b893e Compare December 14, 2023 09:00
@mversic mversic requested a review from Erigara December 14, 2023 10:18
@mversic mversic force-pushed the fix_peer_registration branch from 87b893e to 5f83e2c Compare December 14, 2023 10:25
Signed-off-by: Marin Veršić <marin.versic101@gmail.com>
Signed-off-by: Marin Veršić <marin.versic101@gmail.com>
@mversic mversic force-pushed the fix_peer_registration branch from 5f83e2c to 4bf47f6 Compare December 14, 2023 11:02
@VAmuzing VAmuzing self-assigned this Dec 14, 2023
@mversic mversic merged commit 4765612 into hyperledger-iroha:iroha2-dev Dec 14, 2023
12 checks passed
@mversic mversic deleted the fix_peer_registration branch December 14, 2023 11:58
Asem-Abdelhady pushed a commit to Asem-Abdelhady/iroha that referenced this pull request Jan 9, 2024
…er-iroha#4142)

Signed-off-by: Marin Veršić <marin.versic101@gmail.com>
Asem-Abdelhady pushed a commit to Asem-Abdelhady/iroha that referenced this pull request Jan 22, 2024
…er-iroha#4142)

Signed-off-by: Marin Veršić <marin.versic101@gmail.com>
Signed-off-by: Asem-Abdelhady <asemshawkey@gmail.com>
Asem-Abdelhady pushed a commit to Asem-Abdelhady/iroha that referenced this pull request Feb 9, 2024
…er-iroha#4142)

Signed-off-by: Marin Veršić <marin.versic101@gmail.com>
# for free to join this conversation on GitHub. Already have an account? # 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.

4 participants