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

Disallow empty Identifiers #2000

Closed
mversic opened this issue Mar 23, 2022 · 2 comments
Closed

Disallow empty Identifiers #2000

mversic opened this issue Mar 23, 2022 · 2 comments
Assignees
Labels
good first issue Good for newcomers help wanted Extra attention is needed iroha2-dev The re-implementation of a BFT hyperledger in RUST

Comments

@mversic
Copy link
Contributor

mversic commented Mar 23, 2022

At the moment it is allowed to create empty Name, DomainId, AccountId, etc. Should we continue to allow empty string identifiers or should we forbid them. What about using Option?

It should be noted that empty string is a well defined identifier

@mversic mversic added iroha2-dev The re-implementation of a BFT hyperledger in RUST help wanted Extra attention is needed labels Mar 23, 2022
@appetrosyan appetrosyan added the good first issue Good for newcomers label May 4, 2022
@ales-tsurko ales-tsurko self-assigned this Jun 2, 2022
@ales-tsurko
Copy link
Contributor

@mversic @appetrosyan could you clarify this issue?

It should be noted that empty string is a well defined identifier

So Name type is allowed to have empty string? But in which case AccountId can be empty? And is it possible, for example, to have AccountId like ""@""? So we should require non-empty strings for some types? I mean, should we forbid, for example, using empty name and domainID for AccountId, or forbid name to be empty for DomainId (because it's the only field), but still allow Name to be empty?

@appetrosyan
Copy link
Contributor

To me it seems that the sole purpose of the Name struct is to encapsulate that some valid strings are invalid identifiers. As such an empty identifier is not a valid Name. Thus, we should allow an empty constString, but not an empty Name.

ales-tsurko added a commit to ales-tsurko/iroha that referenced this issue Jun 28, 2022
Signed-off-by: Ales Tsurko <ales.tsurko@gmail.com>
ales-tsurko added a commit to ales-tsurko/iroha that referenced this issue Jun 28, 2022
Signed-off-by: Ales Tsurko <ales.tsurko@gmail.com>
ales-tsurko added a commit to ales-tsurko/iroha that referenced this issue Jun 28, 2022
Signed-off-by: Ales Tsurko <ales.tsurko@gmail.com>
ales-tsurko added a commit to ales-tsurko/iroha that referenced this issue Jun 28, 2022
Signed-off-by: Ales Tsurko <ales.tsurko@gmail.com>
ales-tsurko added a commit to ales-tsurko/iroha that referenced this issue Jun 28, 2022
Signed-off-by: Ales Tsurko <ales.tsurko@gmail.com>
ales-tsurko added a commit to ales-tsurko/iroha that referenced this issue Jun 28, 2022
Signed-off-by: Ales Tsurko <ales.tsurko@gmail.com>
ales-tsurko added a commit to ales-tsurko/iroha that referenced this issue Jun 28, 2022
Signed-off-by: Ales Tsurko <ales.tsurko@gmail.com>
ales-tsurko added a commit to ales-tsurko/iroha that referenced this issue Jun 28, 2022
Signed-off-by: Ales Tsurko <ales.tsurko@gmail.com>
ales-tsurko added a commit that referenced this issue Jun 28, 2022
* [feature] #2000: Disallow empty names

Signed-off-by: Ales Tsurko <ales.tsurko@gmail.com>

* [feature] #2000: Remove dead code

Signed-off-by: Ales Tsurko <ales.tsurko@gmail.com>

* [feature] #2000: Fix grammar mistakes

Signed-off-by: Ales Tsurko <ales.tsurko@gmail.com>

* [feature] #2000: Update Name tests

Signed-off-by: Ales Tsurko <ales.tsurko@gmail.com>
appetrosyan pushed a commit that referenced this issue Jul 4, 2022
* [feature] #2000: Disallow empty names

Signed-off-by: Ales Tsurko <ales.tsurko@gmail.com>

* [feature] #2000: Remove dead code

Signed-off-by: Ales Tsurko <ales.tsurko@gmail.com>

* [feature] #2000: Fix grammar mistakes

Signed-off-by: Ales Tsurko <ales.tsurko@gmail.com>

* [feature] #2000: Update Name tests

Signed-off-by: Ales Tsurko <ales.tsurko@gmail.com>
BAStos525 pushed a commit to BAStos525/soramitsu-iroha that referenced this issue Jul 8, 2022
…roha#2403)

* [feature] hyperledger-iroha#2000: Disallow empty names

Signed-off-by: Ales Tsurko <ales.tsurko@gmail.com>

* [feature] hyperledger-iroha#2000: Remove dead code

Signed-off-by: Ales Tsurko <ales.tsurko@gmail.com>

* [feature] hyperledger-iroha#2000: Fix grammar mistakes

Signed-off-by: Ales Tsurko <ales.tsurko@gmail.com>

* [feature] hyperledger-iroha#2000: Update Name tests

Signed-off-by: Ales Tsurko <ales.tsurko@gmail.com>
Signed-off-by: BAStos525 <jungle.vas@yandex.ru>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed iroha2-dev The re-implementation of a BFT hyperledger in RUST
Projects
None yet
Development

No branches or pull requests

3 participants