nns: simplify checks on split, use different errors

1. splitAndCheck doesn't need allowMultipleFragments since there is exactly one user of it and it checks for the number of fragments anyway. 2. We can panic right inside of it with a bit more details, nil fragments are never valid. 3. Except for one case in checkRecord where the error must be different, so safeSplitAndCheck is introduced. Fixes #411. Signed-off-by: Roman Khimov <[email protected]>
nspcc-dev · Jul 29, 2024 · efd3137 · efd3137
1 parent 906305a
commit efd3137
Show file tree

Hide file tree

Showing 4 changed files with 31 additions and 28 deletions.
diff --git a/contracts/nns/contract.go b/contracts/nns/contract.go
@@ -301,10 +301,7 @@ func GetPrice() int {
 // IsAvailable checks whether the provided domain name is available. Notice that
 // TLD is available for the committee only.
 func IsAvailable(name string) bool {
-	fragments := splitAndCheck(name, true)
-	if fragments == nil {
-		panic("invalid domain name format")
-	}
+	fragments := splitAndCheck(name)
 	ctx := storage.GetReadOnlyContext()
 	l := len(fragments)
 	if storage.Get(ctx, append([]byte{prefixRoot}, []byte(fragments[l-1])...)) == nil {
@@ -367,10 +364,7 @@ func parentExpired(ctx storage.Context, first int, fragments []string) bool {
 //   - starting from the 3rd level, the domain can only be registered by the
 //     owner or administrator (if any) of the previous level domain
 func Register(name string, owner interop.Hash160, email string, refresh, retry, expire, ttl int) bool {
-	fragments := splitAndCheck(name, true)
-	if fragments == nil {
-		panic("invalid domain name format")
-	}
+	fragments := splitAndCheck(name)
 
 	l := len(fragments)
 	if l == 1 {
@@ -436,9 +430,9 @@ func RegisterTLD(name, email string, refresh, retry, expire, ttl int) {
 // record and saves domain state calling saveDomain with given parameters and
 // empty owner. The name MUST be a valid TLD name.
 func saveCommitteeDomain(ctx storage.Context, name, email string, refresh, retry, expire, ttl int) {
-	fragments := splitAndCheck(name, false)
+	fragments := splitAndCheck(name)
 	if len(fragments) != 1 {
-		panic("invalid domain name format")
+		panic("not a TLD")
 	}
 
 	tldKey := makeTLDKey(name)
@@ -535,7 +529,8 @@ func checkRecord(ctx storage.Context, name string, typ recordtype.Type, data str
 	case recordtype.A:
 		ok = checkIPv4(data)
 	case recordtype.CNAME:
-		ok = splitAndCheck(data, true) != nil
+		_, msg := safeSplitAndCheck(data)
+		ok = len(msg) == 0
 	case recordtype.TXT:
 		ok = len(data) <= maxTXTRecordLength
 	case recordtype.AAAA:
@@ -884,23 +879,31 @@ func isAlNum(c uint8) bool {
 	return c >= 'a' && c <= 'z' || c >= '0' && c <= '9'
 }
 
-// splitAndCheck splits domain name into parts and validates it.
-func splitAndCheck(name string, allowMultipleFragments bool) []string {
+// safeSplitAndCheck validates the name and splits it into parts, if anything
+// is wrong it returns a non-empty string in the second result.
+func safeSplitAndCheck(name string) ([]string, string) {
 	l := len(name)
 	if l < minDomainNameLength || maxDomainNameLength < l {
-		return nil
+		return nil, "invalid domain name length"
 	}
 	fragments := std.StringSplit(name, ".")
 	l = len(fragments)
-	if l > 2 && !allowMultipleFragments {
-		return nil
-	}
 	for i := 0; i < l; i++ {
 		if !checkFragment(fragments[i], i == l-1) {
-			return nil
+			return nil, "invalid domain fragment"
 		}
 	}
-	return fragments
+	return fragments, ""
+}
+
+// splitAndCheck splits domain name into parts and validates it. It panics
+// if anything is wrong.
+func splitAndCheck(name string) []string {
+	r, err := safeSplitAndCheck(name)
+	if len(err) != 0 {
+		panic(err)
+	}
+	return r
 }
 
 // checkIPv4 checks record on IPv4 compliance.
@@ -1015,10 +1018,7 @@ func checkIPv6(data string) bool {
 
 // tokenIDFromName returns token ID (domain.root) from the provided name.
 func tokenIDFromName(ctx storage.Context, name string) string {
-	fragments := splitAndCheck(name, true)
-	if fragments == nil {
-		panic("invalid domain name format")
-	}
+	fragments := splitAndCheck(name)
 
 	sum := 0
 	l := len(fragments) - 1

diff --git a/contracts/nns/contract.nef b/contracts/nns/contract.nef
diff --git a/contracts/nns/manifest.json b/contracts/nns/manifest.json
@@ -1 +1 @@
-{"name":"NameService","abi":{"methods":[{"name":"_initialize","offset":0,"parameters":[],"returntype":"Void","safe":false},{"name":"_deploy","offset":32,"parameters":[{"name":"data","type":"Any"},{"name":"isUpdate","type":"Boolean"}],"returntype":"Void","safe":false},{"name":"addRecord","offset":3253,"parameters":[{"name":"name","type":"String"},{"name":"typ","type":"Integer"},{"name":"data","type":"String"}],"returntype":"Void","safe":false},{"name":"balanceOf","offset":872,"parameters":[{"name":"owner","type":"Hash160"}],"returntype":"Integer","safe":true},{"name":"decimals","offset":677,"parameters":[],"returntype":"Integer","safe":true},{"name":"deleteRecords","offset":3571,"parameters":[{"name":"name","type":"String"},{"name":"typ","type":"Integer"}],"returntype":"Void","safe":false},{"name":"getAllRecords","offset":3817,"parameters":[{"name":"name","type":"String"}],"returntype":"InteropInterface","safe":true},{"name":"getPrice","offset":1323,"parameters":[],"returntype":"Integer","safe":true},{"name":"getRecords","offset":3485,"parameters":[{"name":"name","type":"String"},{"name":"typ","type":"Integer"}],"returntype":"Array","safe":true},{"name":"isAvailable","offset":1357,"parameters":[{"name":"name","type":"String"}],"returntype":"Boolean","safe":true},{"name":"ownerOf","offset":699,"parameters":[{"name":"tokenID","type":"ByteArray"}],"returntype":"Hash160","safe":true},{"name":"properties","offset":769,"parameters":[{"name":"tokenID","type":"ByteArray"}],"returntype":"Map","safe":true},{"name":"register","offset":1810,"parameters":[{"name":"name","type":"String"},{"name":"owner","type":"Hash160"},{"name":"email","type":"String"},{"name":"refresh","type":"Integer"},{"name":"retry","type":"Integer"},{"name":"expire","type":"Integer"},{"name":"ttl","type":"Integer"}],"returntype":"Boolean","safe":false},{"name":"registerTLD","offset":2361,"parameters":[{"name":"name","type":"String"},{"name":"email","type":"String"},{"name":"refresh","type":"Integer"},{"name":"retry","type":"Integer"},{"name":"expire","type":"Integer"},{"name":"ttl","type":"Integer"}],"returntype":"Void","safe":false},{"name":"renew","offset":2574,"parameters":[{"name":"name","type":"String"}],"returntype":"Integer","safe":false},{"name":"resolve","offset":3755,"parameters":[{"name":"name","type":"String"},{"name":"typ","type":"Integer"}],"returntype":"Array","safe":true},{"name":"roots","offset":1217,"parameters":[],"returntype":"InteropInterface","safe":true},{"name":"setAdmin","offset":2785,"parameters":[{"name":"name","type":"String"},{"name":"admin","type":"Hash160"}],"returntype":"Void","safe":false},{"name":"setPrice","offset":1245,"parameters":[{"name":"price","type":"Integer"}],"returntype":"Void","safe":false},{"name":"setRecord","offset":2960,"parameters":[{"name":"name","type":"String"},{"name":"typ","type":"Integer"},{"name":"id","type":"Integer"},{"name":"data","type":"String"}],"returntype":"Void","safe":false},{"name":"symbol","offset":671,"parameters":[],"returntype":"String","safe":true},{"name":"tokens","offset":948,"parameters":[],"returntype":"InteropInterface","safe":true},{"name":"tokensOf","offset":977,"parameters":[{"name":"owner","type":"Hash160"}],"returntype":"InteropInterface","safe":true},{"name":"totalSupply","offset":683,"parameters":[],"returntype":"Integer","safe":true},{"name":"transfer","offset":1039,"parameters":[{"name":"to","type":"Hash160"},{"name":"tokenID","type":"ByteArray"},{"name":"data","type":"Any"}],"returntype":"Boolean","safe":false},{"name":"update","offset":587,"parameters":[{"name":"nef","type":"ByteArray"},{"name":"manifest","type":"String"},{"name":"data","type":"Any"}],"returntype":"Void","safe":false},{"name":"updateSOA","offset":2695,"parameters":[{"name":"name","type":"String"},{"name":"email","type":"String"},{"name":"refresh","type":"Integer"},{"name":"retry","type":"Integer"},{"name":"expire","type":"Integer"},{"name":"ttl","type":"Integer"}],"returntype":"Void","safe":false},{"name":"version","offset":679,"parameters":[],"returntype":"Integer","safe":true}],"events":[{"name":"Transfer","parameters":[{"name":"from","type":"Hash160"},{"name":"to","type":"Hash160"},{"name":"amount","type":"Integer"},{"name":"tokenId","type":"ByteArray"}]}]},"features":{},"groups":[],"permissions":[{"contract":"0xfffdc93764dbaddd97c48f252a53ea4643faa3fd","methods":["update"]},{"contract":"*","methods":["onNEP11Payment"]}],"supportedstandards":["NEP-11"],"trusts":[],"extra":null}
+{"name":"NameService","abi":{"methods":[{"name":"_initialize","offset":0,"parameters":[],"returntype":"Void","safe":false},{"name":"_deploy","offset":32,"parameters":[{"name":"data","type":"Any"},{"name":"isUpdate","type":"Boolean"}],"returntype":"Void","safe":false},{"name":"addRecord","offset":3135,"parameters":[{"name":"name","type":"String"},{"name":"typ","type":"Integer"},{"name":"data","type":"String"}],"returntype":"Void","safe":false},{"name":"balanceOf","offset":872,"parameters":[{"name":"owner","type":"Hash160"}],"returntype":"Integer","safe":true},{"name":"decimals","offset":677,"parameters":[],"returntype":"Integer","safe":true},{"name":"deleteRecords","offset":3453,"parameters":[{"name":"name","type":"String"},{"name":"typ","type":"Integer"}],"returntype":"Void","safe":false},{"name":"getAllRecords","offset":3699,"parameters":[{"name":"name","type":"String"}],"returntype":"InteropInterface","safe":true},{"name":"getPrice","offset":1323,"parameters":[],"returntype":"Integer","safe":true},{"name":"getRecords","offset":3367,"parameters":[{"name":"name","type":"String"},{"name":"typ","type":"Integer"}],"returntype":"Array","safe":true},{"name":"isAvailable","offset":1357,"parameters":[{"name":"name","type":"String"}],"returntype":"Boolean","safe":true},{"name":"ownerOf","offset":699,"parameters":[{"name":"tokenID","type":"ByteArray"}],"returntype":"Hash160","safe":true},{"name":"properties","offset":769,"parameters":[{"name":"tokenID","type":"ByteArray"}],"returntype":"Map","safe":true},{"name":"register","offset":1775,"parameters":[{"name":"name","type":"String"},{"name":"owner","type":"Hash160"},{"name":"email","type":"String"},{"name":"refresh","type":"Integer"},{"name":"retry","type":"Integer"},{"name":"expire","type":"Integer"},{"name":"ttl","type":"Integer"}],"returntype":"Boolean","safe":false},{"name":"registerTLD","offset":2291,"parameters":[{"name":"name","type":"String"},{"name":"email","type":"String"},{"name":"refresh","type":"Integer"},{"name":"retry","type":"Integer"},{"name":"expire","type":"Integer"},{"name":"ttl","type":"Integer"}],"returntype":"Void","safe":false},{"name":"renew","offset":2485,"parameters":[{"name":"name","type":"String"}],"returntype":"Integer","safe":false},{"name":"resolve","offset":3637,"parameters":[{"name":"name","type":"String"},{"name":"typ","type":"Integer"}],"returntype":"Array","safe":true},{"name":"roots","offset":1217,"parameters":[],"returntype":"InteropInterface","safe":true},{"name":"setAdmin","offset":2670,"parameters":[{"name":"name","type":"String"},{"name":"admin","type":"Hash160"}],"returntype":"Void","safe":false},{"name":"setPrice","offset":1245,"parameters":[{"name":"price","type":"Integer"}],"returntype":"Void","safe":false},{"name":"setRecord","offset":2832,"parameters":[{"name":"name","type":"String"},{"name":"typ","type":"Integer"},{"name":"id","type":"Integer"},{"name":"data","type":"String"}],"returntype":"Void","safe":false},{"name":"symbol","offset":671,"parameters":[],"returntype":"String","safe":true},{"name":"tokens","offset":948,"parameters":[],"returntype":"InteropInterface","safe":true},{"name":"tokensOf","offset":977,"parameters":[{"name":"owner","type":"Hash160"}],"returntype":"InteropInterface","safe":true},{"name":"totalSupply","offset":683,"parameters":[],"returntype":"Integer","safe":true},{"name":"transfer","offset":1039,"parameters":[{"name":"to","type":"Hash160"},{"name":"tokenID","type":"ByteArray"},{"name":"data","type":"Any"}],"returntype":"Boolean","safe":false},{"name":"update","offset":587,"parameters":[{"name":"nef","type":"ByteArray"},{"name":"manifest","type":"String"},{"name":"data","type":"Any"}],"returntype":"Void","safe":false},{"name":"updateSOA","offset":2593,"parameters":[{"name":"name","type":"String"},{"name":"email","type":"String"},{"name":"refresh","type":"Integer"},{"name":"retry","type":"Integer"},{"name":"expire","type":"Integer"},{"name":"ttl","type":"Integer"}],"returntype":"Void","safe":false},{"name":"version","offset":679,"parameters":[],"returntype":"Integer","safe":true}],"events":[{"name":"Transfer","parameters":[{"name":"from","type":"Hash160"},{"name":"to","type":"Hash160"},{"name":"amount","type":"Integer"},{"name":"tokenId","type":"ByteArray"}]}]},"features":{},"groups":[],"permissions":[{"contract":"0xfffdc93764dbaddd97c48f252a53ea4643faa3fd","methods":["update"]},{"contract":"*","methods":["onNEP11Payment"]}],"supportedstandards":["NEP-11"],"trusts":[],"extra":null}
diff --git a/tests/nns_test.go b/tests/nns_test.go
@@ -80,9 +80,12 @@ func TestNNSRegisterTLD(t *testing.T) {
 
 	refresh, retry, expire, ttl := int64(101), int64(102), int64(103), int64(104)
 
-	c.InvokeFail(t, "invalid domain name format", "registerTLD",
+	c.InvokeFail(t, "invalid domain fragment", "registerTLD",
 		"0com", "[email protected]", refresh, retry, expire, ttl)
 
+	c.InvokeFail(t, "not a TLD", "registerTLD",
+		"neo.org", "[email protected]", refresh, retry, expire, ttl)
+
 	acc := c.NewAccount(t)
 	cAcc := c.WithSigners(acc)
 	cAcc.InvokeFail(t, "not witnessed by committee", "registerTLD",
@@ -108,10 +111,10 @@ func TestNNSRegister(t *testing.T) {
 
 	c3 := c.WithSigners(accTop, acc)
 	t.Run("domain names with hyphen", func(t *testing.T) {
-		c3.InvokeFail(t, "invalid domain name format", "register",
+		c3.InvokeFail(t, "invalid domain fragment", "register",
 			"-testdomain.com", acc.ScriptHash(),
 			"[email protected]", refresh, retry, expire, ttl)
-		c3.InvokeFail(t, "invalid domain name format", "register",
+		c3.InvokeFail(t, "invalid domain fragment", "register",
 			"testdomain-.com", acc.ScriptHash(),
 			"[email protected]", refresh, retry, expire, ttl)
 		c3.Invoke(t, true, "register",
@@ -411,7 +414,7 @@ func TestNNSResolve(t *testing.T) {
 	records := stackitem.NewArray([]stackitem.Item{stackitem.Make("expected result")})
 	c.Invoke(t, records, "resolve", "test.com", int64(recordtype.TXT))
 	c.Invoke(t, records, "resolve", "test.com.", int64(recordtype.TXT))
-	c.InvokeFail(t, "invalid domain name format", "resolve", "test.com..", int64(recordtype.TXT))
+	c.InvokeFail(t, "invalid domain fragment", "resolve", "test.com..", int64(recordtype.TXT))
 
 	// Check CNAME is properly resolved and is not included into the result list.
 	c.Invoke(t, stackitem.NewArray([]stackitem.Item{stackitem.Make("1.2.3.4"), stackitem.Make("5.6.7.8")}), "resolve", "test.com", int64(recordtype.A))