NameService: minor improvements, part 2 #25
Conversation
AnnaShaleva
force-pushed
the
nns-upd-2
branch
from
20d34ea to
0c57843
Compare
|
I'll rebase it to the current master in a few minutes to omit merged commits. |
AnnaShaleva
force-pushed
the
nns-upd-2
branch
from
0c57843 to
f397dc8
Compare
|
Ready for review. |
| ByteString buffer = nameMap[GetKey(name)]; | ||
| if (buffer is null) return true; | ||
| NameState token = (NameState)StdLib.Deserialize(buffer); | ||
| if (Runtime.Time >= token.Expiration) return true; |
There was a problem hiding this comment.
Is not better to store the min expiration time into NameState, seems expensive to me the method getNameState
There was a problem hiding this comment.
Then on every Renew we have to iterate over the whole set of dependant token children and re-calculate minimum expiration time for each NameState which implies retrieving the whole set of parent domains. It can be rather expensive too.
There was a problem hiding this comment.
I'd expect there to be ~2-3 iterations typically which doesn't seem to be a lot. Many of the affected methods are safe (OwnerOf, Properties, IsAvailable, etc.), so they'll be mostly invoked in test mode, not on-chain. Non-safe affected methods can have more problems with the alternative layout as with the Renew case above.
I think NNS doesn't have a concept of subdomain, all domain will be xxx.neo |
|
|
I think it's too complicated. |
|
Well, that's the way proper DNS is and it solves some real-world problems. At the same time, maybe we can move 1891f40 out of this set, because IIUC the second patch doesn't raise any questions? |
No, he can't, because it's set as |
|
I think these are improvement and rework of the structure, we need discuss it further more because design has been changed. I guess we can iterate these changes later after release first version. 4a769b1 This one is more critical, we can apply it ASAP. |
Yeah, that's also changed in #29, sorry, we've got three NNS contracts currently with a number of PRs. Check out coredns/coredns@master...nspcc-dev:coredns:master also, this one works with NeoFS version of NNS. |
Originally implemented in nspcc-dev/neofs-contract@14fc086.
And refactor GetRecords/GetAllRecords along the way, move commonly used code to a separate function. Originally introduced in nspcc-dev/neofs-contract@432c02a.
AnnaShaleva
force-pushed
the
nns-upd-2
branch
from
5110a61 to
32def81
Compare
|
Rebased onto the current master (8d72b92). |
The second part of NNS contract update, depends on #23, waiting for the first part to be merged.
Includes: