Info

Download ZIP (156.3 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

33
by rating

Issues

Magic Python Nov 4, 2019 at 10:46
Multisig wallet:
plus:
Very good and well-thought solution. Almost all known problems are solved.
Good speed (0.069G for a transfer with n=16, k=10)
Tests.
It is possible to add a signature to order without modifying seqno.
A root signature is used to sign the whole message.
accept_message() is called only after one signature was verified.
Uses uniq_token to protect from the usage of signatures in different accounts with intersecting keys.

minus:
Garbage collection does a full scan.
There is no protection from a malicious owner of a smart contract.
11
Magic Python Oct 25, 2019 at 13:54
Tried to send one order with 10 signatures (n=16, k=10)
The message was processed and accepted but no out message was set.
EQAK_xCGDyTcKWKQ8DmHS18Q28iOAR4jfDPTZp-KF4YcnBUO
10
Mellow Squid Oct 26, 2019 at 08:09
Thanks for checking!
`show_general_info` shows that contract has threshold of 16 signatures for order authorization, while only has 10 keys. Order with id 1 is signed by 10 signatures but still it is less than 16, thus this order should not be authorized and sent to network. My notations of N and K (N - threshold, K - number of keys) may be confusing, sorry, but --help output is correct.
I wrote simple fift script which simultaneously generates 'multisig-init' query and `order-with-10signatures` query for `old multisig` (that one tested, but without header signature covering whole message): https://pastebin.com/jp4CqDK1 it is deployed on kQBgXSxRRaeUAUcKx-j7_ti6kP5xHZW7gtuDiQrSPQHdcOst (it is just combination of already presented scripts for easy deploy)
Another one for `new-multisig` (that one without some fif-scripts, but with correct signing) https://pastebin.com/uDExDxNT and deployed here 0QDP8rTa4bRM3RRtlLaftRe_fFrewWek8lABcfHDsPCe61XB
Clever Turkey Nov 4, 2019 at 19:33
Manual DNS:
+ Ability to send batched updates.
+ Ability to completely delete expired domains.
+ Support for storing of a domain and its subdomains.
+ Nice tree-like structure.
- There is no way to change smart contract owner.
- When domain is found, but category is not found the returned length must be non-zero.
- Method for getting seqno is named seq_no() instead of standard seqno()..
10
Mellow Squid Nov 4, 2019 at 19:43
There is seq_no() method: mdns.fc ln 153.
I agree with other issues, though.
Clever Turkey Nov 4, 2019 at 19:35
Automatic DNS:
+ Automatic deletion of expired domains.
+ Manual deletion of expired domains.
+ Grace period of 1 month for domain renewal.
+ Support for storing of a domain and its subdomains.
+ Domain sanity checks.
+ Arbitrary domain expiration time.
+ Bigger price for better domain names.
+ Ability to receive donations.
- Due to shadowed res variable registration_cost will succeed with 0 prices, if domain is already registered. This allows to immediately expire any registered domain.
- garbage_collection execution time is not limited in any way, so it can easily exhaust gas limit if there are a lot of expired domains.
- add_expiration_date works in linear time on a number of domains expired in a given second.
- There is no way to customize domain fees.
- calc_cells doesn't account for intermediate dictionary tree cells and their data.
10
Mellow Squid Nov 4, 2019 at 19:59
>Due to shadowed res variable registration_cost will succeed with 0 prices, if domain is already registered....
Don't get it. As far as I can see if domain is already registered, first element of the registration_cost output will be 0 and register_domain will not be executed at all and domain table will not be updated.
>dnsresolve behaves badly if a partial match is found, but there is no category -1....
Sorry, don't get it as well. Are you talking about the case when we ask dnsresolve for nested domain like "a.b.c.d" and that in this case we always should resolve only "a" and then redirect to "a"[-1] subcontract? If so it was not clear from the task and my solution which simultaneously may be used for both redirection of all subdomains and resolving multilevel subdomain in one contract is hardly may be considered flawed. If you are talking about some bugs in dnsresolve sorry for the misunderstanding. Agree about other issues, thank you for review.
Clever Turkey Nov 4, 2019 at 20:12
#issue9041
>As far as I can see if domain is already registered, first element of the registration_cost output will be 0
This is not true, because there are two variables named "res" and outer one is returned and it is still -1. Look:
> int res = -1;
> (slice subdomain, slice prefix, slice remaining, int res) = domain_table.find_prefix?(domain_name, 1016);

>my solution which simultaneously may be used for both redirection of all subdomains and resolving multilevel subdomain in one contract is hardly may be considered flawed

Consider that someone registers "com" without category -1 and "telegram.org". If one would look for "telegram.org.com", then dnsresolve would return result for "telegram.org". You can store only domain with common fixed suffix in a smart contract, so it is always wrong to repeat partial request in the same smart contract.
10
Mellow Squid Nov 4, 2019 at 20:27
>res = -1 issue
Thank you for explanation, you are right. `int` in domain_table lookup is a bug.
> problem with unset -1 in nested domains.
As far as I can see dnsresolve will not return result for `telegram.org` for `telegram.org.com` lookup: in adns.fc ln 409 domain_table is replaced by subdomain_table (i didn't repeat the bug with res from issue above). That way either dns_record will be returned (if subdomain exist), or redirect will be returned (if not full path exist but -1 is set) or (0, null_cell() )will be returned if only part of the path is registered but -1 is not set (correct answer for such case was not described in the task, so I came with that).
Clever Turkey Nov 4, 2019 at 20:42
#issue9048
I agree with you about dnsresolve behavior, so I reflected this in my initial comments.

Also added some other advantages there.
10
Manual DNS:

- Auto accepts message if seqno is 0 without checking signature; malicious party can drain account with messages with seqno=0. Additionally, if init message carries some useful message, it will be lost without error.

- does not have change owner method
Mellow Squid Oct 19, 2019 at 14:43
Hi! Thank you for review, but your are wrong.
External messages are accepted without any checks if and only if stored in the persistent storage seq_no is equal to 0. It is intended behaviour.
We use message with seq_no = 0 to initialise contract. Once it is initialised, any other messages with seq_no=0 will be rejected.
The same way it is not expected that init message will carry any information, it serves pure goal to initialise contract.
Any other dns records may be added either directly during initial persistent storage formation either in subsequent messages.
Manual DNS: no way to change owner public key.

Automatic DNS:
- prices are hardcoded in code and cannot be defined by user
- same goes for basic domain lifetime
- storage price does not account for data bits and for cell trees inside value (references)
- actually, it does not look at values at all,
- domain prolong does not require authorization
Mellow Squid Oct 22, 2019 at 07:02
Hi! Thank you for review. While your notes are correct, I don't agree that most of them are issues:
>prices and periods are hardcoded< Indeed, however since there were no instructions about it in contest description, development of such methods would be senseless (in any case owner of DNS will change both prices and methods which control them)
>storage price does not account cell innards at all< This is a good take. I expect that special opcode which calculates full size of the cell will appear in the near future. As for now, calculation of number of dns records are good enough. If somebody are 100% sure that dns-record should fit in one cell, she always can keep only cell, without references, but again no instructions were given for this (highly case specific) matter.
>domain prolong does not require authorization< It is intended behaviour as well: it will facilitate crowd-funding for maintaining resources important for community.
Magic Python Oct 30, 2019 at 06:32
Am I right, that "upgrade_contract" is not protected from replay by seqno? Or the intention was to upgrade data(with seqno) from the message too?
Mellow Squid Oct 30, 2019 at 09:33
Yes `Upgrade contract` should set new persistent storage with seqno by itself. If we will set `next` seqno automatically after upgrade it will impose restrictions on upgrades: for instance it will be impossible to change layout of seqno. Indeed, even if we will change code, still old recv_internal handler will finish, and incremented seq_no will be stored in `old place` whether it is correct for new layout or not.
Nobody added any issues yet...