Info

Download ZIP (16.6 KB)

Testing and Issues

You can test this entry and submit issues during the testing period of the Blockchain Contest contest.

Entries with serious issues will not be able to win the contest, but even minor issues might be important for overall results.

Voting

44

Comments

Manual DNS TON smart contract. Implemented with prefix trees. Author: Adoriasoft.com
You have not added any comments yet...
by rating

Issues

Clever Turkey Nov 4, 2019 at 19:26
Manual DNS:
- There is no way to completely delete expired domains.
- It is much safer to call accept_message() just after the signature is checked.
- Category is 8-bit instead of 16-bit.
- Category 0 should return dict instead of Cell.
- When domain is found, but category is not found the returned length must be non-zero.
10
Nice Llama Nov 5, 2019 at 08:28
"There is no way to completely delete expired domains"
There is a way. One could set the DNS record to some predefined value eg zero byte and consider it as deleted. Yet this issue is out of the task range.

"It is much safer to call accept_message() just after the signature is checked"
If we stay within gas credit it does not mater. Accepting message later is safer since there is a risk of an invalid message.

"Category is 8-bit instead of 16-bit"
Yes. A bug while decoding the message. Contact storage uses 16 bit.
dns-manual-contract.fc:51 category=-2 (owner) or other negative categories are valid, no need to throw

dns-manual-contract.fc:126 category=0 should just return dnsrecord_dict; this code tries to pack everything as bits in one slice, which will overflow with just 4 entries. also same issue with categories <= -2

dns entries and subdomains can't be deleted, which is an issue because if you have "a.b" registered, and want to instead have "a.b.c" and "a.b.d", you are out of luck ("a.b" is a prefix of a new key)
Nice Llama Oct 18, 2019 at 23:11
"category=-2 (owner)" - owner category only makes sense for autoDNS. In the manual DNS we have a separate owner field.
"category=0 should just return dnsrecord_dict" - correct. It was easier to debug as cell. One-liner fix though.
"instead have "a.b.c" and "a.b.d" - I do not see an issue here. "a.b.c" and "a.b.d" will be added as dict records in the "a.b" prefix.
Looking at both fc and fif code there is no ∅ character handling and suffixing at all: this way if you register dom you would not be able to register domain, because dom is prefix of domain. Neither fif script or func does add the ∅ (or any other) suffix to the subdomain name to prevent this collision. This also at the same time violates the 8m+1 rule in the response.

Rejecting negative categories also does not correspond to specification, any signed 16-bit number shall be accepted.

Message acception is executed very far in message handling logic, in case of large prefix tree and/or large category dictionary and/or gas credit decrease contract may cease to process controlling messages.
Nice Llama Oct 21, 2019 at 18:07
"Looking at both fc and fif code there is no ∅ character handling and suffixing at all" - when testing I was able to add test.com.abc domain while having test.com. Did you actually check that longer domain is rejected if shorter prefix exists? Anyway owner is free to add trailing zero on his side.

"Message acception is executed very far in message handling logic" - I didn't see any gas cost estimates for the prefix trees. Are you sure accessing them gets more expensive while tree is growing?
Okay, I have tried to re-test stuff at testnet. How nice that the contract with matching key is already deployed and is clean.
While testing found out another bug: dnsresolve(www.test.com, 1) returns (96, Cell), okay. But using category 2 returns 0 length (should return 96).

Also, the thing that you tried does not work with your contract.
Seqno is increased, but the new entry is not in the tree.
You are ignoring the fact that pfxdict_set may silently fail actually.
From the screenshots we can see that after registering www.test.com, www.test.com.abc silently fails registration.
Moreover, we can see from the tree that there is really no such entry.

You can inspect the transaction here: https://test.ton.org/testnet/transaction?account=0f9Tm2SKKVs0X1W_4F1B7oyNinPacm5PePpF-K2IP9W_ttko&lt=1179565000001&hash=F2233B6C15253B36CE450B969A380354DF2B49C623193C9CE8753801BB35E97E
Nice Llama Oct 21, 2019 at 23:05
Thank you for thorougful testing. For some reason runvm allowed me to add strings with the same prefix hence my approach to the contract. So we can stick with owner manually adding the trailing zeroes to the domains.

"But using category 2 returns 0 length (should return 96)." - correct. This requirement is buried quite deep in the description. I assumed that not found category is the same is not found domain but now I see that it is wrong.
It is strange that for you vm behaviour differs so much. In my contract i did negative testing on that situation and it expectedly failed. Maybe you forgot to persist data between vm calls or somehow throwed in data with no shared prefix.
Clever Turkey Nov 5, 2019 at 08:59
#issue9039
> One could set the DNS record to some predefined value eg zero byte and consider it as deleted. Yet this issue is out of the task range.
This is not "completely delete". The domain will be stored forever and the contract will forever pay for it, eventually going out of money. This is a critical issue. "The compactness" and "the efficiency" are very significant properties. More over prefix trees can't store two strings one of which is a prefix of another, so the contract has described by Huge Giraffe problem, affecting correctness.
Nice Llama Nov 5, 2019 at 10:01
"More over prefix trees can't store two strings one of which is a prefix of another"
They cannot use such strings as keys. My solution is to allow client to add zero bytes delimiters.
This feature of the prefix trees is unrelated to deleting DNS records. One can delete or overwrite record without changing the dictionary keys.
Clever Turkey Nov 5, 2019 at 08:59
#issue9039
> If we stay within gas credit it does not mater.
Of course, but you can't predict how much gas will be needed for pfxdict_get/pfxdict_set, because it depends on the size of the dictionary. There is a high chance that someday the contract will stop working for some long domains.
> Accepting message later is safer since there is a risk of an invalid message.
No, there is no risk of invalid message, because seqno and owner's signature are checked already.
Clever Turkey Nov 5, 2019 at 10:15
#issue9054
If client will add zero byte at the end and keep dots inside domain, then none of the string will be ever prefix of another, so -1 category handling in dnsresolve is useless. All dots must be replaced with zeros to make your contract to work as expected, making "com\0google\0" a prefix of "com\0google\0www\0".
Nice Llama Nov 5, 2019 at 12:27
Yes. This is what proposed as a workaround for the reported issue.
Nobody added any issues yet...