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] #4212: Prevent account registration without signatures #4309

Merged

Conversation

Arjentix
Copy link
Contributor

Linked issue

Benefits

More logical

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 Feb 20, 2024
@Arjentix Arjentix self-assigned this Feb 20, 2024
@coveralls
Copy link
Collaborator

coveralls commented Feb 20, 2024

Pull Request Test Coverage Report for Build 7993950372

Details

  • -35 of 129 (72.87%) changed or added relevant lines in 15 files are covered.
  • 2965 unchanged lines in 54 files lost coverage.
  • Overall coverage decreased (-0.3%) to 56.532%

Changes Missing Coverage Covered Lines Changed/Added Lines %
cli/src/lib.rs 0 1 0.0%
client_cli/src/main.rs 0 1 0.0%
core/src/queue.rs 10 12 83.33%
smart_contract/src/lib.rs 0 2 0.0%
core/src/smartcontracts/isi/account.rs 0 5 0.0%
data_model/src/account.rs 30 54 55.56%
Files with Coverage Reduction New Missed Lines %
core/src/sumeragi/network_topology.rs 1 98.78%
data_model/src/account.rs 1 73.48%
tools/kagami/src/crypto.rs 1 12.12%
data_model/src/metadata.rs 2 78.87%
primitives/src/unique_vec.rs 2 91.24%
config/src/kura.rs 3 80.0%
config/src/logger.rs 5 72.73%
logger/src/lib.rs 5 94.12%
tools/kagami/src/main.rs 5 0.0%
crypto/src/signature/secp256k1.rs 7 96.09%
Totals Coverage Status
Change from base Build 7884695009: -0.3%
Covered Lines: 22454
Relevant Lines: 39719

💛 - Coveralls

data_model/src/account.rs Outdated Show resolved Hide resolved
data_model/src/account.rs Outdated Show resolved Hide resolved
data_model/src/account.rs Outdated Show resolved Hide resolved
data_model/src/account.rs Outdated Show resolved Hide resolved
data_model/src/account.rs Outdated Show resolved Hide resolved
data_model/src/account.rs Outdated Show resolved Hide resolved
data_model/src/account.rs Show resolved Hide resolved
@s8sato s8sato self-assigned this Feb 21, 2024
@Arjentix Arjentix force-pushed the prevent_accounts_without_keys branch from 183ea07 to 6fd45d2 Compare February 21, 2024 18:38
@github-actions github-actions bot added config-changes Changes in configuration and start up of the Iroha and removed api-changes Changes in the API for client libraries labels Feb 21, 2024
Copy link

@BAStos525

@Arjentix
Copy link
Contributor Author

@BAStos525

Wow, something new... But why was it triggered? I haven't changed any configs...

@s8sato
Copy link
Contributor

s8sato commented Feb 22, 2024

#4309 (comment)

This is a new feature introduced by #4276 to automatically label and notify someone of specific code changes.
Currently config-changes reacts to any change under configs/, which may be an inappropriate pattern.
I guess configs/*.template.* would be correct one at the current directory structure.
Since things may have changed after #4239 I'd hear your opinion @0x009922
For this topic let's move on to #4278 (comment)

Apart from that we observed the bot removed the manual api-changes label, which will be problematic.
2024-02-22T125101+090000
This is because with the current setting, it always checks the consistency between the changed files and the labeler configuration.
I'll disable this synchronization in another PR #4316

data_model/src/account.rs Outdated Show resolved Hide resolved
@Arjentix
Copy link
Contributor Author

Currently config-changes reacts to any change under configs/, which may be an inappropriate pattern.

Yes, we change executor.wasm in pretty much every PR. And I'm sure it doesn't affect the way we deploy Iroha.

@Arjentix Arjentix force-pushed the prevent_accounts_without_keys branch from 6fd45d2 to b28f02f Compare February 23, 2024 14:15
@Arjentix Arjentix force-pushed the prevent_accounts_without_keys branch from b28f02f to 8d55b9e Compare February 23, 2024 14:48
@Arjentix Arjentix merged commit 41c08b4 into hyperledger:iroha2-dev Feb 26, 2024
10 of 12 checks passed
@Arjentix Arjentix deleted the prevent_accounts_without_keys branch February 26, 2024 12:04
@s8sato s8sato added api-changes Changes in the API for client libraries and removed config-changes Changes in configuration and start up of the Iroha labels Feb 26, 2024
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.

4 participants