Mention domain in the error message in NNS #88

Merged
fyrchik merged 1 commit from elebedeva/frostfs-contract:fix/domain-error-msg into master 2024-04-16 15:53:07 +00:00
2 changed files with 20 additions and 16 deletions

View file

@ -231,15 +231,20 @@ func IsAvailable(name string) bool {
} }
return true return true
} }
if parentExpired(ctx, fragments) { checkParentExists(ctx, fragments)
panic("parent does not exist or is expired")
}
return storage.Get(ctx, append([]byte{prefixName}, getTokenKey([]byte(name))...)) == nil return storage.Get(ctx, append([]byte{prefixName}, getTokenKey([]byte(name))...)) == nil
  1. parentExpired checks all parents in chain, it would be nice to know exactly which domain does not exist. We can achieve this either by returning index in fragments or replacing parentExpired with checkParentExist inside the function. All parentExpired usages panic anyway.
  2. Similar panic is used in 2 other place, should be handled in a similar fashion.
1. `parentExpired` checks all parents in chain, it would be nice to know exactly which domain does not exist. We can achieve this either by returning index in fragments or replacing `parentExpired` with `checkParentExist` inside the function. All `parentExpired` usages panic anyway. 2. Similar panic is used in 2 other place, should be handled in a similar fashion.

Replaced all parentExpired checks with checkParentExist.

Replaced all `parentExpired` checks with `checkParentExist`.
} }
// parentExpired returns true if any domain from fragments doesn't exist or is expired. // 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 != "" {
panic("domain does not exist or is expired: " + dom)
}
}
// parentExpired returns domain from fragments that doesn't exist or is expired.
// first denotes the deepest subdomain to check. // first denotes the deepest subdomain to check.
func parentExpired(ctx storage.Context, fragments []string) bool { func parentExpired(ctx storage.Context, fragments []string) string {
now := int64(runtime.GetTime()) now := int64(runtime.GetTime())
last := len(fragments) - 1 last := len(fragments) - 1
name := fragments[last] name := fragments[last]
@ -249,14 +254,14 @@ func parentExpired(ctx storage.Context, fragments []string) bool {
} }
nsBytes := storage.Get(ctx, append([]byte{prefixName}, getTokenKey([]byte(name))...)) nsBytes := storage.Get(ctx, append([]byte{prefixName}, getTokenKey([]byte(name))...))
if nsBytes == nil { if nsBytes == nil {
return true return name
} }
ns := std.Deserialize(nsBytes.([]byte)).(NameState) ns := std.Deserialize(nsBytes.([]byte)).(NameState)
if now >= ns.Expiration { if now >= ns.Expiration {
return true return name
} }
} }
return false return ""
} }
// Register registers a new domain with the specified owner and name if it's available. // Register registers a new domain with the specified owner and name if it's available.
@ -280,9 +285,8 @@ func Register(name string, owner interop.Hash160, email string, refresh, retry,
if tldBytes == nil { if tldBytes == nil {
panic("TLD not found") panic("TLD not found")
} }
if parentExpired(ctx, fragments) { checkParentExists(ctx, fragments)
panic("one of the parent domains is not registered")
}
parentKey := getTokenKey([]byte(name[len(fragments[0])+1:])) parentKey := getTokenKey([]byte(name[len(fragments[0])+1:]))
nsBytes := storage.Get(ctx, append([]byte{prefixName}, parentKey...)) nsBytes := storage.Get(ctx, append([]byte{prefixName}, parentKey...))
ns := std.Deserialize(nsBytes.([]byte)).(NameState) ns := std.Deserialize(nsBytes.([]byte)).(NameState)
@ -511,9 +515,7 @@ func getNameState(ctx storage.Context, tokenID []byte) NameState {
tokenKey := getTokenKey(tokenID) tokenKey := getTokenKey(tokenID)
ns := getNameStateWithKey(ctx, tokenKey) ns := getNameStateWithKey(ctx, tokenKey)
fragments := std.StringSplit(string(tokenID), ".") fragments := std.StringSplit(string(tokenID), ".")
if parentExpired(ctx, fragments) { checkParentExists(ctx, fragments)
panic("parent domain has expired")
}
return ns return ns
} }

View file

@ -167,7 +167,7 @@ func TestNNSRegisterMulti(t *testing.T) {
c1 := c.WithSigners(acc) c1 := c.WithSigners(acc)
t.Run("parent domain is missing", func(t *testing.T) { t.Run("parent domain is missing", func(t *testing.T) {
msg := "one of the parent domains is not registered" msg := "domain does not exist or is expired: fs.neo.com"
args[0] = "testnet.fs.neo.com" args[0] = "testnet.fs.neo.com"
c1.InvokeFail(t, msg, "register", args...) c1.InvokeFail(t, msg, "register", args...)
}) })
@ -342,6 +342,8 @@ func TestNNSIsAvailable(t *testing.T) {
acc := c.NewAccount(t) acc := c.NewAccount(t)
c1 := c.WithSigners(c.Committee, acc) c1 := c.WithSigners(c.Committee, acc)
c1.InvokeFail(t, "domain does not exist or is expired: domain.com", "isAvailable", "dom.domain.com")
c1.Invoke(t, true, "register", c1.Invoke(t, true, "register",
"domain.com", acc.ScriptHash(), "domain.com", acc.ScriptHash(),
"myemail@frostfs.info", refresh, retry, expire, ttl) "myemail@frostfs.info", refresh, retry, expire, ttl)
@ -349,7 +351,7 @@ func TestNNSIsAvailable(t *testing.T) {
c.Invoke(t, false, "isAvailable", "domain.com") c.Invoke(t, false, "isAvailable", "domain.com")

Could you also add this test before this line?

	c1.Invoke(t, true, "register",
		"domain.com", acc.ScriptHash(),
		"myemail@frostfs.info", refresh, retry, expire, ttl)

The error should be the same, but with a different domain ("domain.com").

Could you also add this test before this line? ``` c1.Invoke(t, true, "register", "domain.com", acc.ScriptHash(), "myemail@frostfs.info", refresh, retry, expire, ttl) ``` The error should be the same, but with a different domain ("domain.com").

Added.

Added.
c.Invoke(t, true, "isAvailable", "dom.domain.com") c.Invoke(t, true, "isAvailable", "dom.domain.com")
c.InvokeFail(t, "parent does not exist or is expired", "isAvailable", "dom.dom.domain.com") c.InvokeFail(t, "domain does not exist or is expired: dom.domain.com", "isAvailable", "dom.dom.domain.com")
c1.Invoke(t, true, "register", c1.Invoke(t, true, "register",
"dom.domain.com", acc.ScriptHash(), "dom.domain.com", acc.ScriptHash(),