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.

Sign up for GitHub

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

Jump to bottom

[feature] #3090: Propagate on-chain parameters #3457

Merged
merged 2 commits into from
May 11, 2023
Merged

[feature] #3090: Propagate on-chain parameters #3457

merged 2 commits into from
May 11, 2023

Conversation

Erigara
Copy link
Contributor

@Erigara Erigara commented May 5, 2023 •
edited
Loading

Description

  • Set clear boundary between sumeragi interface (handle) and sumeragi itself
  • Propagate on-chain parameters

Linked issue

Closes #3090

Benefits

Consensus critical configuration params now controlled by consensus, thus no forks due to divergences in this parameters.

Drawbacks

We will try to update parameters after every committed block, but block commit is not that frequent event so it should be fine.

Yet another RefCell inside WorldStateView.

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

@Erigara Erigara self-assigned this May 5, 2023
@github-actions github-actions bot added the iroha2-dev The re-implementation of a BFT hyperledger in RUST label May 5, 2023
@codecov
Copy link

codecov bot commented May 5, 2023 •
edited
Loading

Codecov Report

Merging #3457 (7e37e5b) into iroha2-dev (a4d5c9f) will decrease coverage by 5.63%.
The diff coverage is 31.97%.

@@              Coverage Diff               @@
##           iroha2-dev    #3457      +/-   ##
==============================================
- Coverage       62.33%   56.70%   -5.63%     
==============================================
  Files             169      159      -10     
  Lines           31218    30314     -904     
==============================================
- Hits            19459    17190    -2269     
- Misses          11759    13124    +1365
Impacted Files Coverage Δ
cli/src/lib.rs 0.57% <0.00%> (-68.86%) ⬇️
cli/src/main.rs 0.66% <0.00%> (-0.44%) ⬇️
cli/src/samples.rs 0.00% <0.00%> (-61.85%) ⬇️
cli/src/style.rs 0.00% <0.00%> (ø)
cli/src/torii/mod.rs 0.00% <0.00%> (-27.66%) ⬇️
cli/src/torii/routing.rs 0.00% <0.00%> (-57.11%) ⬇️
cli/src/torii/utils.rs 0.00% <0.00%> (-84.85%) ⬇️
client/src/http_default.rs 0.00% <0.00%> (-38.94%) ⬇️
client_cli/src/main.rs 0.25% <0.00%> (+<0.01%) ⬆️
config/base/src/lib.rs 95.08% <ø> (+58.71%) ⬆️
... and 100 more

... and 53 files with indirect coverage changes

@appetrosyan appetrosyan self-assigned this May 5, 2023
@ilchu ilchu self-assigned this May 5, 2023
appetrosyan
appetrosyan reviewed May 7, 2023
View reviewed changes
cli/src/lib.rs Outdated Show resolved Hide resolved
appetrosyan
appetrosyan reviewed May 7, 2023
View reviewed changes
cli/src/lib.rs Outdated Show resolved Hide resolved
cli/src/torii/routing.rs Show resolved Hide resolved
cli/src/torii/routing.rs Show resolved Hide resolved
client/tests/integration/connected_peers.rs Outdated Show resolved Hide resolved
client/tests/integration/multiple_blocks_created.rs Outdated Show resolved Hide resolved
core/src/sumeragi/main_loop.rs Outdated Show resolved Hide resolved
core/src/sumeragi/mod.rs Show resolved Hide resolved
core/src/sumeragi/mod.rs Show resolved Hide resolved
core/src/sumeragi/mod.rs Outdated Show resolved Hide resolved
tools/kagami/src/main.rs Outdated Show resolved Hide resolved
@Erigara Erigara mentioned this pull request May 10, 2023
Closed
@Erigara Erigara force-pushed the propagate_params branch 3 times, most recently from 2fe79f1 to 5ca633b Compare May 10, 2023 14:16
@appetrosyan appetrosyan mentioned this pull request May 10, 2023
Closed
mversic
mversic reviewed May 10, 2023
View reviewed changes
data_model/src/lib.rs Outdated Show resolved Hide resolved
ilchu
ilchu previously approved these changes May 11, 2023
View reviewed changes
Copy link
Contributor

@ilchu ilchu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stellar work, glad to see the parameters finally doing their thing.

config/src/sumeragi.rs Outdated Show resolved Hide resolved
config/src/sumeragi.rs Outdated Show resolved Hide resolved
core/src/sumeragi/main_loop.rs Outdated Show resolved Hide resolved
client/benches/tps/utils.rs Outdated Show resolved Hide resolved
@Arjentix Arjentix self-assigned this May 11, 2023
@Erigara
[refactor]: Add SumeragiHandle
b2fe6e1 
Signed-off-by: Shanin Roman <shanin1000@yandex.ru>
appetrosyan
appetrosyan previously approved these changes May 11, 2023
View reviewed changes
Arjentix
Arjentix reviewed May 11, 2023
View reviewed changes
cli/src/torii/routing.rs Show resolved Hide resolved
client/tests/integration/asset_propagation.rs Outdated Show resolved Hide resolved
client/tests/integration/connected_peers.rs Outdated Show resolved Hide resolved
client/tests/integration/offline_peers.rs Outdated Show resolved Hide resolved
configs/peer/genesis.json Show resolved Hide resolved
configs/peer/genesis.json Show resolved Hide resolved
data_model/src/transaction.rs Show resolved Hide resolved
core/src/wsv.rs Outdated Show resolved Hide resolved
core/src/wsv.rs Outdated Show resolved Hide resolved
core/src/wsv.rs Show resolved Hide resolved
@Erigara
[feature] hyperledger-iroha#3090: Propagate on-chain parameters
7e37e5b 
Signed-off-by: Shanin Roman <shanin1000@yandex.ru>
@Erigara Erigara dismissed stale reviews from appetrosyan and ilchu via 7e37e5b May 11, 2023 14:00
@Erigara Erigara merged commit d7e054d into hyperledger-iroha:iroha2-dev May 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Arjentix Arjentix Arjentix left review comments

@mversic mversic mversic left review comments

@appetrosyan appetrosyan appetrosyan approved these changes

@ilchu ilchu ilchu approved these changes

@s8sato s8sato Awaiting requested review from s8sato

@SamHSmith SamHSmith Awaiting requested review from SamHSmith

@QuentinI QuentinI Awaiting requested review from QuentinI

@outoftardis outoftardis Awaiting requested review from outoftardis

@pesterev pesterev Awaiting requested review from pesterev

@0x009922 0x009922 Awaiting requested review from 0x009922

Assignees

@appetrosyan appetrosyan

@ilchu ilchu

@Arjentix Arjentix

@Erigara Erigara

Labels
iroha2-dev The re-implementation of a BFT hyperledger in RUST
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Erigara @appetrosyan @ilchu @Arjentix @mversic