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

No description provided.

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

[#xxxx] policy: Add constants for more targets

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

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

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

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>

[#xxxx] policy: Test very large entity name

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

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

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

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.

fyrchik commented

2024-12-12 12:39:56 +00:00

Closing stale PR, feel free to reeopen.

fyrchik closed this pull request

2024-12-12 12:39:57 +00:00