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

[feature] #3231: Monolithic validator #3329

Merged
merged 1 commit into from
Apr 20, 2023

Conversation

Arjentix
Copy link
Contributor

@Arjentix Arjentix commented Mar 24, 2023

Description

  • No more chain of validators in core. Just one monolithic validator
  • Genesis new requires validator
  • Added Upgrade instruction
  • Recursive transaction and instruction validation moved to validator

Previously we checked instructions of one tx on Iroha side. Now it's a work for Runtime Permission Validator. However Runtime Validator still can accept Instruction as input, because it's required for WASM smartcontracts and triggers.

Transaction instructions need to be aplied to wsv in order to properly validate them (if one instrucion depends on another). This forces us to execute instructions on validator. It's kind of unexpected for a thing called Validator. So @appetrosyan suggests to rename it to Verifier, which will accept WSV and return the updated one. But I haven't done that in this PR, because such semantic change requires a lot of changes in all code base, including block.rs, main_loop.rs and etc.

I wonder what others think about that.

Linked issue

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

@Arjentix Arjentix added Enhancement New feature or request iroha2-dev The re-implementation of a BFT hyperledger in RUST api-changes Changes in the API for client libraries labels Mar 24, 2023
@Arjentix Arjentix self-assigned this Mar 24, 2023
@Arjentix
Copy link
Contributor Author

I'll squash commits into one when will be ready to merge

@codecov
Copy link

codecov bot commented Mar 24, 2023

Codecov Report

Merging #3329 (7a45693) into iroha2-dev (a4d5c9f) will decrease coverage by 4.57%.
The diff coverage is 45.76%.

❗ Current head 7a45693 differs from pull request most recent head 031aa7b. Consider uploading reports for the commit 031aa7b to get more accurate results

@@              Coverage Diff               @@
##           iroha2-dev    #3329      +/-   ##
==============================================
- Coverage       62.33%   57.76%   -4.57%     
==============================================
  Files             169      162       -7     
  Lines           31218    31865     +647     
==============================================
- Hits            19459    18408    -1051     
- Misses          11759    13457    +1698     
Impacted Files Coverage Δ
cli/src/lib.rs 0.28% <0.00%> (-69.15%) ⬇️
cli/src/main.rs 0.85% <0.00%> (-0.25%) ⬇️
cli/src/samples.rs 0.00% <0.00%> (-61.85%) ⬇️
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 91.57% <ø> (+55.21%) ⬆️
config/src/lib.rs 33.33% <ø> (ø)
... and 103 more

... and 28 files with indirect coverage changes

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

@appetrosyan appetrosyan self-assigned this Mar 24, 2023
configs/peer/config.json Outdated Show resolved Hide resolved
wasm/Cargo.toml Outdated Show resolved Hide resolved
tools/kagami/Cargo.toml Outdated Show resolved Hide resolved
data_model/src/events/data/events.rs Outdated Show resolved Hide resolved
data_model/src/events/data/events.rs Outdated Show resolved Hide resolved
data_model/src/lib.rs Outdated Show resolved Hide resolved
data_model/src/permission/mod.rs Outdated Show resolved Hide resolved
data_model/src/transaction.rs Outdated Show resolved Hide resolved
wasm/validator/src/recursive/isi.rs Outdated Show resolved Hide resolved
schema/gen/src/lib.rs Show resolved Hide resolved
wasm/src/debug.rs Outdated Show resolved Hide resolved
data_model/src/lib.rs Outdated Show resolved Hide resolved
data_model/src/lib.rs Show resolved Hide resolved
data_model/src/lib.rs Outdated Show resolved Hide resolved
data_model/src/permission/validator.rs Outdated Show resolved Hide resolved
data_model/src/permission/validator.rs Show resolved Hide resolved
data_model/src/permission/validator.rs Outdated Show resolved Hide resolved
core/src/smartcontracts/isi/mod.rs Outdated Show resolved Hide resolved
wasm/validator/src/recursive/isi.rs Outdated Show resolved Hide resolved
wasm/validator/src/recursive/isi.rs Outdated Show resolved Hide resolved
wasm/validator/src/recursive/isi.rs Outdated Show resolved Hide resolved
core/src/smartcontracts/wasm.rs Outdated Show resolved Hide resolved
@mversic mversic self-assigned this Mar 28, 2023
core/src/smartcontracts/wasm.rs Outdated Show resolved Hide resolved
core/src/smartcontracts/wasm.rs Show resolved Hide resolved
core/src/smartcontracts/wasm.rs Outdated Show resolved Hide resolved
core/src/validator.rs Outdated Show resolved Hide resolved
data_model/src/events/data/events.rs Show resolved Hide resolved
genesis/src/lib.rs Show resolved Hide resolved
wasm/validator/derive/src/entrypoint.rs Show resolved Hide resolved
wasm/validator/src/recursive/isi.rs Outdated Show resolved Hide resolved
wasm/validator/src/recursive/mod.rs Outdated Show resolved Hide resolved
data_model/src/events/data/events.rs Show resolved Hide resolved
data_model/src/permission/validator.rs Outdated Show resolved Hide resolved
genesis/src/lib.rs Show resolved Hide resolved
wasm/validator/src/recursive/isi.rs Outdated Show resolved Hide resolved
core/src/validator.rs Outdated Show resolved Hide resolved
mversic
mversic previously approved these changes Apr 19, 2023
core/src/wsv.rs Show resolved Hide resolved
core/src/wsv.rs Outdated Show resolved Hide resolved
core/src/validator.rs Outdated Show resolved Hide resolved
core/src/validator.rs Show resolved Hide resolved
Arjentix added a commit to Arjentix/iroha that referenced this pull request Apr 19, 2023
Signed-off-by: Daniil Polyakov <arjentix@gmail.com>
SamHSmith
SamHSmith previously approved these changes Apr 19, 2023
Copy link
Contributor

@SamHSmith SamHSmith left a comment

Choose a reason for hiding this comment

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

LGTM

client/tests/integration/burn_public_keys.rs Outdated Show resolved Hide resolved
mversic
mversic previously approved these changes Apr 19, 2023
appetrosyan
appetrosyan previously approved these changes Apr 20, 2023
Arjentix added a commit to Arjentix/iroha that referenced this pull request Apr 20, 2023
Signed-off-by: Daniil Polyakov <arjentix@gmail.com>
@Arjentix Arjentix dismissed stale reviews from appetrosyan, mversic, and SamHSmith via 03cb786 April 20, 2023 18:51
Arjentix added a commit to Arjentix/iroha that referenced this pull request Apr 20, 2023
Signed-off-by: Daniil Polyakov <arjentix@gmail.com>
Signed-off-by: Daniil Polyakov <arjentix@gmail.com>
@Arjentix Arjentix merged commit 8551f36 into hyperledger:iroha2-dev Apr 20, 2023
@Arjentix Arjentix deleted the monolithic_validator branch April 20, 2023 20:11
appetrosyan pushed a commit that referenced this pull request Jun 5, 2023
Signed-off-by: Daniil Polyakov <arjentix@gmail.com>
mversic pushed a commit that referenced this pull request Oct 17, 2023
Signed-off-by: Daniil Polyakov <arjentix@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-changes Changes in the API for client libraries Enhancement New feature or request iroha2-dev The re-implementation of a BFT hyperledger in RUST
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants