Mention domain in the error message in NNS #88

Merged
fyrchik merged 1 commits 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
}
if parentExpired(ctx, fragments) {
panic("parent does not exist or is expired")
}
checkParentExists(ctx, fragments)
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.
func parentExpired(ctx storage.Context, fragments []string) bool {
func parentExpired(ctx storage.Context, fragments []string) string {
now := int64(runtime.GetTime())
last := len(fragments) - 1
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))...))
if nsBytes == nil {
return true
return name
}
ns := std.Deserialize(nsBytes.([]byte)).(NameState)
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.
@ -280,9 +285,8 @@ func Register(name string, owner interop.Hash160, email string, refresh, retry,
if tldBytes == nil {
panic("TLD not found")
}
if parentExpired(ctx, fragments) {
panic("one of the parent domains is not registered")
}
checkParentExists(ctx, fragments)
parentKey := getTokenKey([]byte(name[len(fragments[0])+1:]))
nsBytes := storage.Get(ctx, append([]byte{prefixName}, parentKey...))
ns := std.Deserialize(nsBytes.([]byte)).(NameState)
@ -511,9 +515,7 @@ func getNameState(ctx storage.Context, tokenID []byte) NameState {
tokenKey := getTokenKey(tokenID)
ns := getNameStateWithKey(ctx, tokenKey)
fragments := std.StringSplit(string(tokenID), ".")
if parentExpired(ctx, fragments) {
panic("parent domain has expired")
}
checkParentExists(ctx, fragments)
return ns
}

View File

@ -167,7 +167,7 @@ func TestNNSRegisterMulti(t *testing.T) {
c1 := c.WithSigners(acc)
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"
c1.InvokeFail(t, msg, "register", args...)
})
@ -342,6 +342,8 @@ func TestNNSIsAvailable(t *testing.T) {
acc := c.NewAccount(t)
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",
"domain.com", acc.ScriptHash(),
"myemail@frostfs.info", refresh, retry, expire, ttl)
@ -349,7 +351,7 @@ func TestNNSIsAvailable(t *testing.T) {
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.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",
"dom.domain.com", acc.ScriptHash(),