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

refs: Prohibit IDs consisting of all zero bytes #303

Merged
merged 1 commit into from
Jul 22, 2024
Merged

Conversation

cthulhu-rider
Copy link
Contributor

i propose to reserve zero IDs and prohibit them from being message fields. While this is a breaking change in theory, in practice nothing will break. At the same time, application development will benefit

Container and object IDs are SHA-256 checksums of their contents. It is
almost impossible to find data that corresponds to a zero identifier.
Setting 32 zeros rather indicates a client error. Reserving a zero value
will also make application development easier: corresponding type will
have a reserved invalid value within itself. Even if the unimaginable
and some structure will actually have a zero hash - so be it, the
problem is negligible for the final benefit.

Note that zero `OwnerID` is already invalid at least due to the non-zero
prefix. Therefore does not need clarification.

Signed-off-by: Leonard Lyubich <[email protected]>
Copy link
Member

@carpawell carpawell left a comment

Choose a reason for hiding this comment

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

Overall agree. But do we really need to specify it for the fields that are commented as "hash"? What if container's hash is all zeros? It a caller is a liar (and that is not a 32 zeros hash), it is easy to check.

@cthulhu-rider
Copy link
Contributor Author

What if container's hash is all zeros? It a caller is a liar (and that is not a 32 zeros hash), it is easy to check.

we see zeros - we dont hash the container at all. That's the difference

But do we really need to specify it for the fields that are commented as "hash"?

for hash - mb not. But i prefer to explicitly state such cases. For example, although zeros are not a particularly valid hash, the protocol may dictate a special interpretation of them. And so we emphasize that it does not exist

@roman-khimov
Copy link
Member

@carpawell
Copy link
Member

@roman-khimov, I have checked UUID generators we used and it is impossible to get zero ID. At the same time, I do not understand what prevents getting zero sha256.

@roman-khimov
Copy link
Member

The probability of getting it is rather low. Hash collision between two containers (or objects) is more probable than that and if it happens -- we're toast.

@roman-khimov roman-khimov merged commit ec8f1f5 into master Jul 22, 2024
3 checks passed
@roman-khimov roman-khimov deleted the zero-ids branch July 22, 2024 16:45
cthulhu-rider added a commit to nspcc-dev/neofs-sdk-go that referenced this pull request Jul 25, 2024
This follows nspcc-dev/neofs-api#303. `IsZero`
methods and `ErrZero` errors are provided for callers to handle zeros.

Signed-off-by: Leonard Lyubich <[email protected]>
cthulhu-rider added a commit to nspcc-dev/neofs-sdk-go that referenced this pull request Jul 26, 2024
This follows nspcc-dev/neofs-api#303. `IsZero`
methods and `ErrZero` errors are provided for callers to handle zeros.

Signed-off-by: Leonard Lyubich <[email protected]>
cthulhu-rider added a commit to nspcc-dev/neofs-sdk-go that referenced this pull request Jul 26, 2024
This follows nspcc-dev/neofs-api#303. `IsZero`
methods and `ErrZero` errors are provided for callers to handle zeros.

Signed-off-by: Leonard Lyubich <[email protected]>
@roman-khimov roman-khimov added this to the v2.17.0 milestone Oct 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants