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

Account-based EACL targets #278

Closed
roman-khimov opened this issue Nov 22, 2023 · 5 comments · Fixed by #301
Closed

Account-based EACL targets #278

roman-khimov opened this issue Nov 22, 2023 · 5 comments · Fixed by #301
Assignees
Labels
enhancement Improving existing functionality I2 Regular impact S3 Minimally significant U4 Nothing urgent
Milestone

Comments

@roman-khimov
Copy link
Member

Is your feature request related to a problem? Please describe.

I'm always frustrated when I see public keys in the protocol used instead of accounts. Account is a very powerful abstraction that is not limited to a single key. Current EACL target requirement to have public keys leads to problems in the service layer like nspcc-dev/neofs-s3-gw#861 and while workarounds are possible the root cause of the issue is the requirement to have public keys in targets, they should not be needed to process EACL.

Describe the solution you'd like

Extend target definition with a list of accounts. Deprecate key usage.

Describe alternatives you've considered

Reusing the same field is technically possible (it's just bytes). Not sure it's any better, but you can try convincing me.

@cthulhu-rider
Copy link
Contributor

but you can try convincing me

how about: LEN=33 => key, otherwise account?

@roman-khimov
Copy link
Member Author

Huh. That'd be similar to CheckWitness accepting keys or accounts. But it can be done.

@cthulhu-rider
Copy link
Contributor

But it can be done

good. My main question is - can account be 33B in theory?

@roman-khimov
Copy link
Member Author

To me they all 20 bytes (unfortunately, 25 in real life). If for whatever reason we need some new kind of account and it's 33 bytes originally, we can just add a prefix to distinguish it.

@cthulhu-rider
Copy link
Contributor

then i think we can just shift existing field to accounts with the ability to use keys of length 33. Same message format, easier implementation

smallhive added a commit that referenced this issue Jul 5, 2024
Closes #278.

Signed-off-by: Evgenii Baidakov <evgenii@nspcc.io>
smallhive added a commit that referenced this issue Jul 26, 2024
Closes #278.

Signed-off-by: Evgenii Baidakov <evgenii@nspcc.io>
smallhive added a commit that referenced this issue Jul 26, 2024
Closes #278.

Signed-off-by: Evgenii Baidakov <evgenii@nspcc.io>
@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
enhancement Improving existing functionality I2 Regular impact S3 Minimally significant U4 Nothing urgent
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants