WIP: Store hashed entity name in policy contract #87

alexvanin · 2024-04-15T06:13:48Z

alexvanin commented

2024-04-15 06:13:48 +00:00

There is no content yet.

alexvanin added 3 commits 2024-04-15 06:13:50 +00:00

DCO action / DCO (pull_request) Failing after 59s Details

Tests / Tests (1.21) (pull_request) Successful in 1m43s Details

Tests / Tests (1.22) (pull_request) Successful in 1m45s Details

5b0ea5e0a8 [#xxxx] policy: Add constants for more targets

Signed-off-by: Alex Vanin <a.vanin@yadro.com>

670c74610b [#xxxx] policy: Hash entity name to support large namespaces

Namespace is a part of entity name, so it should be hashed
in order to fit neo-go storage key.

Signed-off-by: Alex Vanin <a.vanin@yadro.com>

DCO action / DCO (pull_request) Failing after 1m1s Details

Tests / Tests (1.21) (pull_request) Successful in 1m19s Details

Tests / Tests (1.22) (pull_request) Successful in 1m23s Details

4022e3b6c3 [#xxxx] policy: Test very large entity name

Signed-off-by: Alex Vanin <a.vanin@yadro.com>

fyrchik reviewed 2024-04-15 06:34:14 +00:00

policy/policy_contract.go

													
				@ -105,6 +108,9 @@ func storageKey(prefix Kind, counter int, name []byte) []byte {

				}

				func mapKey(kind Kind, name []byte) []byte {

					if len(name) > 0 {

fyrchik commented

2024-04-15 06:34:03 +00:00

Why do we need this check, can it be false?

alexvanin commented

2024-04-17 07:58:58 +00:00

Poster

Yes, ListTargets uses mapKey with empty name to iterate all storage keys based on kind prefix.

Yes, `ListTargets` uses `mapKey` with empty name to iterate all storage keys based on kind prefix.

alexvanin commented

2024-04-17 07:53:31 +00:00

Poster

We've decided to avoid this change for v0.19.X releases.
I see an issue with proposed approach: ListTargets method becomes useless. It returns hashed sha256(entityName), and this info cannot be reused later. So I see couple of options:

Remove ListTargets and all use-cases with this method (are there any?)
Rework this PR and store tuple of entityKind and entityName as a value in contract, so it can be returned in ListTargets
Something else?

We've decided to avoid this change for `v0.19.X` releases. I see an issue with proposed approach: `ListTargets` method becomes useless. It returns hashed `sha256(entityName)`, and this info cannot be reused later. So I see couple of options: 1) Remove `ListTargets` and all use-cases with this method (are there any?) 2) Rework this PR and store tuple of `entityKind` and `entityName` as a value in contract, so it can be returned in `ListTargets` 3) Something else?

fyrchik commented

2024-04-17 08:03:32 +00:00

Option 2 looks ok to me
So we have mappings:

'v' + entityKind +  sha256(name) -> name
'k' + sha256(name) -> 8-byte id

We can iterate over the first to get a list.
Frankly, I would remove ListTargets unless it is really needed.

Option 2 looks ok to me So we have mappings: ``` 'v' + entityKind + sha256(name) -> name 'k' + sha256(name) -> 8-byte id ``` We can iterate over the first to get a list. Frankly, I would remove `ListTargets` unless it is really needed.

DCO action / DCO (pull_request) Failing after 1m1s

Details

Tests / Tests (1.21) (pull_request) Successful in 1m19s

Details

Tests / Tests (1.22) (pull_request) Successful in 1m23s

Details

This pull request is marked as a work in progress.

This branch is out-of-date with the base branch

Sign in to join this conversation.

No reviewers