nns: Add support global domain #102

Merged
fyrchik merged 2 commits from achuprov/frostfs-contract:feta/global_domain into master 2024-09-04 19:51:18 +00:00
4 changed files with 261 additions and 2 deletions
Showing only changes of commit 82e04b6c32 - Show all commits

View file

@ -11,6 +11,7 @@
| 0x20 | int | set of roots |
| 0x21 + tokenKey | ByteArray | serialized NameState struct |
| 0x22 + tokenKey + Hash160(tokenName) | Hash160 | container contract hash |
| 0x23 + tokenKey + Hash160(tokenName) | string | global domain flag |
*/

View file

@ -41,6 +41,9 @@ const (
// prefixRecord contains map from (token key + hash160(token name) + record type)
// to record.
prefixRecord byte = 0x22
//prefixGlobalDomain contains a flag indicating that this domain was created using GlobalDomain.
//This is necessary to distinguish it from regular CNAME records.
prefixGlobalDomain byte = 0x23
aarifullin marked this conversation as resolved Outdated

As you have introduced a new key in the contract storage, please, make it mentioned in the scheme table in doc.go

As you have introduced a new key in the contract storage, please, make it mentioned in the scheme table in `doc.go`

Thanks, fixed

Thanks, fixed
)
// Values constraints.
@ -69,6 +72,12 @@ const (
errInvalidDomainName = "invalid domain name format"
)
const (
// Cnametgt is a special TXT record ensuring all created subdomains point to the global domain - the value of this variable.
//It is guaranteed that two domains cannot point to the same global domain.
Cnametgt = "cnametgt"
)
// RecordState is a type that registered entities are saved to.
type RecordState struct {
Name string
@ -230,10 +239,71 @@ func IsAvailable(name string) bool {
}
return true
}
checkParentExists(ctx, fragments)
achuprov marked this conversation as resolved Outdated

Both usages of check panic on failure.
Can we panic inside? See checkParentExists, this is an ok practice in contracts.

Both usages of `check` panic on failure. Can we panic inside? See `checkParentExists`, this is an ok practice in contracts.

fixed

fixed
checkAvailableGlobalDomain(ctx, name)
achuprov marked this conversation as resolved Outdated

See #98

It would be nice to mention this global domain name.

See https://git.frostfs.info/TrueCloudLab/frostfs-contract/pulls/98 It would be nice to mention this global domain name.

fixed

fixed
return storage.Get(ctx, append([]byte{prefixName}, getTokenKey([]byte(name))...)) == nil
}
// checkAvailableGlobalDomain - triggers a panic if the global domain name is occupied.

Why do we check for CNAME only and with id=0?
It seems the right way is to check for any records.

Why do we check for `CNAME` only and with `id=0`? It seems the right way is to check for any records.

Also, if we have a domain b.c and record a.b.c, will we behave correctly?

Also, if we have a domain `b.c` and record `a.b.c`, will we behave correctly?
func checkAvailableGlobalDomain(ctx storage.Context, domain string) {
achuprov marked this conversation as resolved Outdated

same here

same here

fixed

fixed
globalDomain := getGlobalDomain(ctx, domain)
if globalDomain == "" {
return
}
nsBytes := storage.Get(ctx, append([]byte{prefixName}, getTokenKey([]byte(globalDomain))...))
if nsBytes != nil {
panic("global domain is already taken: " + globalDomain + ". Domain: " + domain)
}
achuprov marked this conversation as resolved Outdated

useless

useless

Fixd

Fixd
}
// getGlobalDomain returns the global domain.
func getGlobalDomain(ctx storage.Context, domain string) string {
index := std.MemorySearch([]byte(domain), []byte("."))
achuprov marked this conversation as resolved Outdated

Doc-comment should start with a function name (s/G/g/)

Doc-comment should start with a function name (`s/G/g/`)

fixed

fixed
if index == -1 {
return ""
}
achuprov marked this conversation as resolved Outdated

You first split string, and then build it again.
It could be more beneficial (in terms of code size and spent GAS) to just strip the prefix from the whole string.

You first split string, and then build it again. It could be more beneficial (in terms of code size and spent GAS) to just strip the prefix from the whole string.

fixed

fixed
name := domain[index+1:]
if name == "" {
return ""
}
achuprov marked this conversation as resolved Outdated

I don't like this whole Global thing being seen in the NNS interface.
In was defined in terms of other NNS features, we should be able to work with it via an already existing interface.

I don't like this whole `Global` thing being seen in the NNS interface. In was defined in terms of other NNS features, we should be able to work with it via an already existing interface.

Fixed

Fixed
return extractCnametgt(ctx, name, domain)
}
// extractCnametgt returns the value of the Cnametgt TXT record.
func extractCnametgt(ctx storage.Context, name, domain string) string {
fragments := splitAndCheck(domain)
tokenID := []byte(tokenIDFromName(name))
records := getRecordsByType(ctx, tokenID, name, TXT)
if records == nil {
return ""
}
globalDomain := ""
for _, name := range records {
fragments := std.StringSplit(name, "=")
if len(fragments) != 2 {
continue
}
if fragments[0] == Cnametgt {
globalDomain = fragments[1]
break
achuprov marked this conversation as resolved Outdated

break here?

`break` here?

fixed

fixed
}
}
if globalDomain == "" {
return ""
}
return fragments[0] + "." + globalDomain
}
// checkParentExists panics if any domain from fragments doesn't exist or is expired.
func checkParentExists(ctx storage.Context, fragments []string) {
if dom := parentExpired(ctx, fragments); dom != "" {
@ -265,10 +335,16 @@ func parentExpired(ctx storage.Context, fragments []string) string {
// Register registers a new domain with the specified owner and name if it's available.
func Register(name string, owner interop.Hash160, email string, refresh, retry, expire, ttl int) bool {
ctx := storage.GetContext()
return register(ctx, name, owner, email, refresh, retry, expire, ttl)
}
// Register registers a new domain with the specified owner and name if it's available.
func register(ctx storage.Context, name string, owner interop.Hash160, email string, refresh, retry, expire, ttl int) bool {
fragments := splitAndCheck(name)
l := len(fragments)
tldKey := append([]byte{prefixRoot}, []byte(fragments[l-1])...)
achuprov marked this conversation as resolved Outdated

The comment is in the wrong place, it was commenting public function.

The comment is in the wrong place, it was commenting public function.

fixed

fixed
ctx := storage.GetContext()
tldBytes := storage.Get(ctx, tldKey)
if l == 1 {
checkCommittee()
@ -325,6 +401,8 @@ func Register(name string, owner interop.Hash160, email string, refresh, retry,
// NNS expiration is in milliseconds
Expiration: int64(runtime.GetTime() + expire*1000),
}
checkAvailableGlobalDomain(ctx, name)
putNameStateWithKey(ctx, tokenKey, ns)
putSoaRecord(ctx, name, email, refresh, retry, expire, ttl)
updateBalance(ctx, []byte(name), owner, +1)
@ -422,6 +500,7 @@ func DeleteRecords(name string, typ RecordType) {
deleteRecords(ctx, name, typ)
}
// DeleteRecords removes domain records with the specified type.
func deleteRecords(ctx storage.Context, name string, typ RecordType) {
if typ == SOA {
panic("you cannot delete soa record")
@ -429,6 +508,14 @@ func deleteRecords(ctx storage.Context, name string, typ RecordType) {
tokenID := []byte(tokenIDFromName(name))
ns := getNameState(ctx, tokenID)
ns.checkAdmin()
globalDomainStorage := append([]byte{prefixGlobalDomain}, getTokenKey([]byte(name))...)
globalDomainRaw := storage.Get(ctx, globalDomainStorage)
globalDomain := globalDomainRaw.(string)
if globalDomainRaw != nil && globalDomain != "" {
deleteDomain(ctx, globalDomain)
}
recordsKey := getRecordsKeyByType(tokenID, name, typ)
records := storage.Find(ctx, recordsKey, storage.KeysOnly)
for iterator.Next(records) {
@ -451,6 +538,12 @@ func deleteDomain(ctx storage.Context, name string) {
return
}
globalDomainRaw := storage.Get(ctx, append([]byte{prefixGlobalDomain}, getTokenKey([]byte(name))...))
globalDomain := globalDomainRaw.(string)
if globalDomainRaw != nil && globalDomain != "" {
deleteDomain(ctx, globalDomain)
}
deleteRecords(ctx, name, CNAME)
deleteRecords(ctx, name, TXT)
deleteRecords(ctx, name, A)
@ -598,6 +691,33 @@ func addRecord(ctx storage.Context, tokenId []byte, name string, typ RecordType,
}
}
globalDomainKey := append([]byte{prefixGlobalDomain}, getTokenKey([]byte(name))...)
globalDomainStorage := storage.Get(ctx, globalDomainKey)
globalDomain := getGlobalDomain(ctx, name)
if globalDomainStorage == nil && typ == TXT {
if globalDomain != "" {
checkAvailableGlobalDomain(ctx, name)
nsOriginal := getNameState(ctx, []byte(tokenIDFromName(name)))
ns := NameState{
Name: globalDomain,
Owner: nsOriginal.Owner,
Expiration: nsOriginal.Expiration,
Admin: nsOriginal.Admin,
}
putNameStateWithKey(ctx, getTokenKey([]byte(globalDomain)), ns)
storage.Put(ctx, globalDomainKey, globalDomain)
var oldOwner interop.Hash160
updateBalance(ctx, []byte(name), nsOriginal.Owner, +1)
postTransfer(oldOwner, nsOriginal.Owner, []byte(name), nil)
putCnameRecord(ctx, globalDomain, name)
} else {
storage.Put(ctx, globalDomainKey, "")
}
}
if typ == CNAME && id != 0 {
panic("you shouldn't have more than one CNAME record")
}
@ -638,6 +758,22 @@ func putSoaRecord(ctx storage.Context, name, email string, refresh, retry, expir
storage.Put(ctx, recordKey, recBytes)
}
// putCnameRecord stores CNAME domain record.
func putCnameRecord(ctx storage.Context, name, data string) {
var id byte
tokenId := []byte(tokenIDFromName(name))
recordKey := getIdRecordKey(tokenId, name, CNAME, id)
rs := RecordState{
Name: name,
Type: CNAME,
ID: id,
Data: data,
}
recBytes := std.Serialize(rs)
storage.Put(ctx, recordKey, recBytes)
}
// updateSoaSerial stores soa domain record.
func updateSoaSerial(ctx storage.Context, tokenId []byte) {
var id byte
@ -645,7 +781,7 @@ func updateSoaSerial(ctx storage.Context, tokenId []byte) {
recBytes := storage.Get(ctx, recordKey)
if recBytes == nil {
panic("not found soa record")
return
}
rec := std.Deserialize(recBytes.([]byte)).(RecordState)

View file

@ -165,6 +165,14 @@ func checkContainerList(t *testing.T, c *neotest.ContractInvoker, expected [][]b
})
}
const (
// default SOA record field values
defaultRefresh = 3600 // 1 hour
defaultRetry = 600 // 10 min
defaultExpire = 3600 * 24 * 365 * 10 // 10 years
defaultTTL = 3600 // 1 hour
)
func TestContainerPut(t *testing.T) {
c, cBal, _ := newContainerInvoker(t)
@ -233,6 +241,63 @@ func TestContainerPut(t *testing.T) {
})
})
t.Run("create global domain", func(t *testing.T) {
ctrNNS := neotest.CompileFile(t, c.CommitteeHash, nnsPath, path.Join(nnsPath, "config.yml"))
nnsHash := ctrNNS.Hash
cNNS := c.CommitteeInvoker(nnsHash)
cNNS.Invoke(t, true, "register",
"animals", c.CommitteeHash,
"whateveriwant@world.com", int64(defaultRefresh), int64(defaultRetry), int64(defaultExpire), int64(defaultTTL))
achuprov marked this conversation as resolved Outdated

defaultSomething instead of magic constants?

`default`Something instead of magic constants?
cNNS.Invoke(t, true, "register",
"ns", c.CommitteeHash,
"whateveriwant@world.com", int64(defaultRefresh), int64(defaultRetry), int64(defaultExpire), int64(0))
cNNS.Invoke(t, true, "register",
"poland.ns", c.CommitteeHash,
"whateveriwant@world.com", int64(defaultRefresh), int64(defaultRetry), int64(defaultExpire), int64(0))
cNNS.Invoke(t, true, "register",
"sweden.ns", c.CommitteeHash,
"whateveriwant@world.com", int64(defaultRefresh), int64(defaultRetry), int64(defaultExpire), int64(defaultExpire))
cNNS.Invoke(t, stackitem.Null{}, "addRecord",
"poland.ns", int64(nns.TXT), nns.Cnametgt+"=animals")
cNNS.Invoke(t, stackitem.Null{}, "addRecord", "poland.ns", int64(nns.TXT), "random-record")
cNNS.Invoke(t, stackitem.Null{}, "addRecord", "poland.ns", int64(nns.TXT), "ne-qqq=random-record2")
cNNS.Invoke(t, stackitem.Null{}, "addRecord", "sweden.ns", int64(nns.TXT), nns.Cnametgt+"=animals")
balanceMint(t, cBal, acc, (containerFee+containerAliasFee)*5, []byte{})
cNNS.Invoke(t, true, "isAvailable", "bober.animals")
putArgs := []any{cnt.value, cnt.sig, cnt.pub, cnt.token, "bober", "poland.ns"}
c3 := c.WithSigners(c.Committee, acc)
c3.Invoke(t, stackitem.Null{}, "putNamed", putArgs...)
cNNS.Invoke(t, false, "isAvailable", "bober.animals")
putArgs = []any{cnt.value, cnt.sig, cnt.pub, cnt.token, "bober", "sweden.ns"}
c3.InvokeFail(t, "global domain is already taken", "putNamed", putArgs...)
cNNS.InvokeFail(t, "global domain is already taken", "isAvailable", "bober.poland.ns")
cnt2 := dummyContainer(acc)
cNNS.Invoke(t, true, "isAvailable", "uzik.poland.ns")
putArgs = []any{cnt2.value, cnt2.sig, cnt2.pub, cnt2.token, "uzik", "poland.ns"}
c3.Invoke(t, stackitem.Null{}, "putNamed", putArgs...)
achuprov marked this conversation as resolved Outdated

Not only we do want to have this record, but we also want it to resolve to something (via our resolve method).
Can we add a tests for this?

Not only we do want to have this record, but we also want it to resolve to something (via our `resolve` method). Can we add a tests for this?

added tests to container_test

added tests to container_test
c3.Invoke(t, stackitem.Null{}, "delete", cnt.id[:], cnt.sig, cnt.pub, cnt.token)
cNNS.Invoke(t, true, "isAvailable", "bober.animals")
cNNS.Invoke(t, false, "isAvailable", "bober.poland.ns")
cNNS.InvokeFail(t, "global domain is already taken", "isAvailable", "uzik.poland.ns")
cNNS.Invoke(t, false, "isAvailable", "uzik.animals")
records := stackitem.NewArray([]stackitem.Item{stackitem.NewBuffer([]byte("uzik.poland.ns")), stackitem.NewByteArray([]byte(base58.Encode(cnt2.id[:])))})
cNNS.Invoke(t, records, "resolve", "uzik.animals", int64(nns.TXT))
})
t.Run("gas costs are the same for all containers in block", func(t *testing.T) {
const (
containerPerBlock = 512

View file

@ -159,6 +159,49 @@ func TestNNSRegister(t *testing.T) {
c.Invoke(t, expected, "getRecords", "testdomain.com", int64(nns.TXT))
}
func TestGlobalDomain(t *testing.T) {
c := newNNSInvoker(t, false)
accTop := c.NewAccount(t)
refresh, retry, expire, ttl := int64(101), int64(102), int64(103), int64(104)
c1 := c.WithSigners(c.Committee, accTop)
c1.Invoke(t, true, "register",
"com", accTop.ScriptHash(),
achuprov marked this conversation as resolved Outdated

Could you move these cases to a separate Test*?
I also do not see the tests for AddRecord -- I believe the behaviour should be similar to register in terms of creating a global domain record.

Could you move these cases to a separate `Test*`? I also do not see the tests for `AddRecord` -- I believe the behaviour should be similar to `register` in terms of creating a global domain record.

Fixed. You are right; however, we can't create SOA records using SetRecord

func checkBaseRecords(typ RecordType, data string) bool {

Fixed. You are right; however, we can't create SOA records using `SetRecord` https://git.frostfs.info/TrueCloudLab/frostfs-contract/src/commit/49e5270f673e228b6b0dc33e1ff41ea7ef7e08b6/nns/nns_contract.go#L382
"myemail@frostfs.info", refresh, retry, expire, ttl)
c1.Invoke(t, true, "register",
"testdomain.com", accTop.ScriptHash(),
"myemail@frostfs.info", refresh, retry, expire, ttl)
c1.Invoke(t, true, "register",
"globaldomain.com", accTop.ScriptHash(),
"myemail@frostfs.info", refresh, retry, expire, ttl)
c1.Invoke(t, true, "register",
"domik.testdomain.com", accTop.ScriptHash(),
"myemail@frostfs.info", refresh, retry, expire, ttl)
c1.Invoke(t, stackitem.Null{}, "addRecord",
"domik.testdomain.com", int64(nns.TXT), "CID")
c.Invoke(t, true, "isAvailable", "domik.globaldomain.com")
achuprov marked this conversation as resolved Outdated

Hmm, why is it available right after we had registered it?

Hmm, why is it available right after we had registered it?

From the RFC: 'When GlobalCNAMEZone is enabled, existing buckets will not be registered in the global zone.' This domain zone was created before GlobalCNAMEZone was enabled. Creating a new TXT record should not affect this.

From the RFC: 'When `GlobalCNAMEZone` is enabled, existing buckets will not be registered in the global zone.' This domain zone was created before `GlobalCNAMEZone` was enabled. Creating a new TXT record should not affect this.
c1.Invoke(t, stackitem.Null{}, "addRecord",
"testdomain.com", int64(nns.TXT), nns.Cnametgt+"=globaldomain.com")
c.Invoke(t, true, "isAvailable", "dom.testdomain.com")
c1.Invoke(t, stackitem.Null{}, "addRecord",
"domik.testdomain.com", int64(nns.TXT), "random txt record")
c.Invoke(t, true, "isAvailable", "domik.globaldomain.com")
c1.Invoke(t, true, "register",
"dom.testdomain.com", accTop.ScriptHash(),
"myemail@frostfs.info", refresh, retry, expire, ttl)
c1.Invoke(t, stackitem.Null{}, "addRecord",
"dom.testdomain.com", int64(nns.TXT), "CID")
c.InvokeFail(t, "global domain is already taken", "isAvailable", "dom.testdomain.com")
}
func TestTLDRecord(t *testing.T) {
c := newNNSInvoker(t, true)
c.Invoke(t, stackitem.Null{}, "addRecord",
@ -365,6 +408,10 @@ func TestNNSIsAvailable(t *testing.T) {
"domain.com", acc.ScriptHash(),
"myemail@frostfs.info", refresh, retry, expire, ttl)
c1.Invoke(t, true, "register",
"globaldomain.com", acc.ScriptHash(),
"myemail@frostfs.info", refresh, retry, expire, ttl)
c.Invoke(t, false, "isAvailable", "domain.com")
c.Invoke(t, true, "isAvailable", "dom.domain.com")
@ -376,6 +423,16 @@ func TestNNSIsAvailable(t *testing.T) {
c.Invoke(t, false, "isAvailable", "dom.domain.com")
c.Invoke(t, true, "isAvailable", "dom.dom.domain.com")
c1.Invoke(t, stackitem.Null{}, "addRecord",
"dom.domain.com", int64(nns.TXT), nns.Cnametgt+"=globaldomain.com")
c.Invoke(t, true, "isAvailable", "dom.dom.domain.com")
c1.Invoke(t, true, "register",
"dom.globaldomain.com", acc.ScriptHash(),
"myemail@frostfs.info", refresh, retry, expire, ttl)
c.InvokeFail(t, "global domain is already taken", "isAvailable", "dom.dom.domain.com")
c.InvokeFail(t, "domain name too long", "isAvailable", getTooLongDomainName(255))
}