TrueCloudLab/frostfs-contract
22
0
Fork 19
Code Issues 12 Pull requests 1 Projects Releases 2 Activity Actions

nns: Add GetDomains #108

Closed
achuprov wants to merge 1 commit from achuprov/frostfs-contract:feat/add_GetSubDomain into master
pull from: achuprov/frostfs-contract:feat/add_GetSubDomain
merge into: TrueCloudLab:master
Conversation 4 Commits 1 Files changed 4 +60 -1
achuprov commented 2024-09-03 15:07:08 +00:00
Member
Copy link

GetDomains returns a list of existing subdomains. After that, it is possible to retrieve a list of all records using GetAllRecords.

Signed-off-by: Alexander Chuprov a.chuprov@yadro.com

`GetDomains` returns a list of existing subdomains. After that, it is possible to retrieve a list of all records using `GetAllRecords`. Signed-off-by: Alexander Chuprov <a.chuprov@yadro.com>
achuprov added 1 commit 2024-09-03 15:07:11 +00:00
[#108] nns: Add GetSubDomains
All checks were successful
DCO action / DCO (pull_request) Successful in 38s
Details
Tests / Tests (1.21) (pull_request) Successful in 1m10s
Details
Tests / Tests (1.22) (pull_request) Successful in 1m10s
Details
e3a353fa75 
Signed-off-by: Alexander Chuprov <a.chuprov@yadro.com>
realloc commented 2024-09-03 15:10:12 +00:00
Owner
Copy link

Semantically, it is more correct to make it possible to receive records of a certain level. In DNS zones, there is no concept of a subdomain as such, there are only records of a certain type. Getting a subdomain at a lower level is just getting records of a zone with a level restriction.

Semantically, it is more correct to make it possible to receive records of a certain level. In DNS zones, there is no concept of a subdomain as such, there are only records of a certain type. Getting a subdomain at a lower level is just getting records of a zone with a level restriction.
achuprov force-pushed feat/add_GetSubDomain from e3a353fa75 to e46ac55b45 2024-09-04 13:58:14 +00:00 Compare
achuprov changed title from nns: Add GetSubDomains to nns: Add GetDomains 2024-09-04 14:04:04 +00:00
achuprov requested review from storage-core-committers 2024-09-04 14:04:13 +00:00
achuprov requested review from storage-core-developers 2024-09-04 14:04:13 +00:00
alexvanin reviewed 2024-09-05 10:50:13 +00:00
tests/nns_test.go Outdated
@ -205,0 +242,4 @@
"myemail@frostfs.info", refresh, retry, expire, ttl)
c1.Invoke(t, []stackitem.Item{stackitem.NewBuffer([]byte("dom.com")), stackitem.NewBuffer([]byte("globaldomain.com")),
stackitem.NewBuffer([]byte("com.com")), stackitem.NewBuffer([]byte("domik.dom.com"))}, "getDomains", "com")
alexvanin commented 2024-09-05 10:50:13 +00:00
Owner
Copy link

I wonder why domik.dom.com and com.com are returned and com.com.com and com.com.com.com are not?

I wonder why `domik.dom.com` and `com.com` are returned and `com.com.com` and `com.com.com.com` are not?
achuprov commented 2024-09-06 09:32:55 +00:00
Author
Member
Copy link

You are right. I've fixed the typo

You are right. I've fixed the typo
achuprov marked this conversation as resolved
achuprov force-pushed feat/add_GetSubDomain from e46ac55b45 to 01a301412d 2024-09-06 08:58:09 +00:00 Compare
dstepanov-yadro approved these changes 2024-09-06 10:38:55 +00:00
Dismissed
fyrchik reviewed 2024-09-06 12:26:54 +00:00
nns/nns.yml Outdated
@ -15,6 +15,7 @@ safemethods:
- "tokens"
- "resolve"
- "roots"
- "getDomains"
fyrchik commented 2024-09-06 12:26:54 +00:00
Owner
Copy link

wrong file

wrong file
achuprov commented 2024-09-06 13:00:07 +00:00
Author
Member
Copy link

fixed

fixed
fyrchik marked this conversation as resolved
achuprov force-pushed feat/add_GetSubDomain from 01a301412d to 79ee453016 2024-09-06 12:33:43 +00:00 Compare
achuprov dismissed dstepanov-yadro's review 2024-09-06 12:33:44 +00:00
Reason:

New commits pushed, approval review dismissed automatically according to repository settings

achuprov force-pushed feat/add_GetSubDomain from 79ee453016 to a5d523b194 2024-09-06 12:34:28 +00:00 Compare
fyrchik reviewed 2024-09-09 07:31:36 +00:00
nns/nns_contract.go Outdated
@ -1067,0 +1083,4 @@
domainRaw := iterator.Value(it).([]byte)
ns := std.Deserialize(domainRaw).(NameState)
firstSeparatorIndex := std.MemorySearch([]byte(ns.Name), []byte("."))
lastSeparatorIndex := std.MemorySearchLastIndex([]byte(ns.Name), []byte(zone), len(ns.Name))
fyrchik commented 2024-09-09 07:31:34 +00:00
Owner
Copy link

Can we exploit ordered iteration in order to prune some ranges?
Currently it is a brute-force with filtering, can (will) break with a sufficiently big number of domains.

Can we exploit ordered iteration in order to prune some ranges? Currently it is a brute-force with filtering, can (will) break with a sufficiently big number of domains.
achuprov commented 2024-09-09 08:40:45 +00:00
Author
Member
Copy link

We use a hash of the domain name to store the NS record, so we can't rely on lexicographical order of iteration.

Lines 618 to 629 in 3e221b9
func getTokenKey(tokenID []byte) []byte {
return crypto.Ripemd160(tokenID)
}
// getNameState returns domain name state by the specified tokenID.
func getNameState(ctx storage.Context, tokenID []byte) NameState {
tokenKey := getTokenKey(tokenID)
ns := getNameStateWithKey(ctx, tokenKey)
fragments := std.StringSplit(string(tokenID), ".")
checkParentExists(ctx, fragments)
return ns
}

We use a hash of the domain name to store the `NS` record, so we can't rely on lexicographical order of iteration. https://git.frostfs.info/TrueCloudLab/frostfs-contract/src/commit/3e221b973a3cfe387113990bab56b33f78bc0c18/nns/nns_contract.go#L618-L629
fyrchik marked this conversation as resolved
achuprov force-pushed feat/add_GetSubDomain from a5d523b194 to 743f082d38 2024-09-09 08:46:29 +00:00 Compare
fyrchik commented 2024-09-10 09:25:40 +00:00
Owner
Copy link

It if doesn't allow to return many items (what is the limit btw?), let's not do this now.
We can return an iterator over all domains (it is in NEP-11 standard) and then filter on the client.
Maybe add a new function which iterates over deserialized values, but still, we should return an iterator.

It if doesn't allow to return many items (what is the limit btw?), let's not do this now. We can return an iterator over all domains (it is in NEP-11 standard) and then filter on the client. Maybe add a new function which iterates over deserialized values, but still, we should return an iterator.
achuprov force-pushed feat/add_GetSubDomain from 743f082d38 to 13c771b038 2024-09-10 12:48:23 +00:00 Compare
achuprov force-pushed feat/add_GetSubDomain from 13c771b038 to bee4c93a56 2024-09-10 12:49:22 +00:00 Compare
achuprov commented 2024-09-10 12:59:36 +00:00
Author
Member
Copy link

Contract-level pagination results in an n^2 complexity, so I suggest we avoid providing deserialized data. Let's return just iterator.

Contract-level pagination results in an `n^2` complexity, so I suggest we avoid providing deserialized data. Let's return just iterator.
aarifullin approved these changes 2024-09-10 15:17:46 +00:00
achuprov commented 2024-09-13 09:25:16 +00:00
Author
Member
Copy link

duplicates the Tokens method.

duplicates the [Tokens](https://git.frostfs.info/TrueCloudLab/frostfs-contract/src/commit/3e221b973a3cfe387113990bab56b33f78bc0c18/nns/nns_contract.go#L165) method.
achuprov closed this pull request 2024-09-13 09:25:17 +00:00
All checks were successful
DCO action / DCO (pull_request) Successful in 25s
Details
Code generation / Generate wrappers (pull_request) Successful in 1m13s
Details
Tests / Tests (pull_request) Successful in 1m18s
Details

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No reviewers
aarifullin
Labels
Clear labels   
P0

Highest

  
P1

High

  
P2

Medium

  
P3

Low

  
good first issue

Good for newcomers

  
Infrastructure

Infrastructure, Automation, CI/CD or DevOps related stuff

  
blocked

The issue or PR is blocked by something

  
bug

Something is not working

  
config

Configuration format update or breaking change

  
discussion

Talking about something in order to reach a decision or to exchange ideas

  
documentation

Documentation related

  
duplicate

This issue or pull request already exists

  
enhancement

New feature

  
go

Language features adoption

  
help wanted

Extra attention is needed

  
internal

   
invalid

Something is wrong

  
kludge

This is a kludge and we know it

  
observability

Related to metrics, tracing and logs

  
perfomance

Perfomance related

  
question

More information is needed

  
refactoring

Refactoring

  
wontfix

This won't be fixed

No labels
P0
P1
P2
P3
good first issue
Infrastructure
blocked
bug
config
discussion
documentation
duplicate
enhancement
go
help wanted
internal
invalid
kludge
observability
perfomance
question
refactoring
wontfix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
6 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: TrueCloudLab/frostfs-contract#108
No description provided.
Delete branch "achuprov/frostfs-contract:feat/add_GetSubDomain"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?