Info

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

75

Comments

Two versions of MultiSig, one is bitcoin-style for external messages, another one works with internal messages sent through participants simple wallets, automatic DNS with pay-per use and possibility of changing storage fee, and a draft of simple asynchronous Payment channel.
1
The main aim of making those solutions was not exactly match the task but find the different, and probably better way to solve the problem. Feel free to comment, give a critics, I will be happy to discuss.
1
You have not added any comments yet...
by rating

Issues

Clever Turkey Nov 5, 2019 at 15:49
Semi-automatic DNS:
+ Manual deletion of expired domains.
+ Support for storing of a domain and its subdomains.
+ Arbitrary domain expiration time.
+ Support for error responses.
+ Ability to receive donations.
+ Ability to withdraw earnings.
- Manual cleaning via full scan is too expensive, this is not a suitable way to implement it.
- Owner check doesn't compare workchain_id.
- Lack of automatic partial resolving on function level.
- All dictionary keys are 256-bit, so there could be high storage costs.
- In set_records difference is substracted from storage, but it needs to be added.
- No update_time change in set_records and topup_domain.
- In some places udict_get? can be used instead of udict_delete_get?.
- Number of cells used by domain doesn't affect price.
- owners dictionary is stored, but not used.
11
Mad Crow Nov 5, 2019 at 17:35
Thank you for the great review! 
- I decided not to compare chain id since it is rather similar to using 256 bits keys, where at least half of bits in hash can be removed. 
- I'm really sorry I had about 30 hours for both DNS and Payment channel, so some things were not tested properly, so submission really has issues with ref cells, and even typos like using subtraction for storage.  
- What about resolving function I really believe it should be done on backend. 
- Bulk deletion is not automated well indeed, and full scan is designed mostly for changing storage price, however deletion is possible for any expired domain by anyone who want to get it. 
- Now I see some ways improve deletion in case we calculate expiration time for each domain, that is an easy option in fact.
- There's a lack of optimization indeed.
Chic Dolphin Oct 21, 2019 at 18:30
> ACCEPT, sets current gas limit to its maximal allowed value, and resets the gas credit to zero. In other words, the current smart contract agrees to buy some gas to finish the current transaction.

You are definitely using accept_message for some other purpose (like 'I will not throw processing it').

You are counting confirmations wrong. I cannot understand yet what exactly that function will produce. But expression updating counter within loop remain constant.
1
Mad Crow Oct 22, 2019 at 06:36
I guess it is necessary to study accept_message issue working with internal messages more careful. During the tests it was ok and if incoming GAS was not enough transaction dropped.

And you are right, here's a typo, right implementation is:

int get_confirmations_count(n, l) {
var counter = 0;
var i = 0;
do {
counter += ((n >> i) & 1);
i += 1;
} until( i == l );
return counter;
}

It shifts confirmations bit mask and count how many signatures are already accepted. I believe we are going to release all the code on GitHub and continue testing and developing. To tell the truth I just want to take a look at native solution by TON before continue.
Shiny Giraffe Nov 3, 2019 at 00:30
I wonder how I did miss this interesting DNS resolver before. While not conforming to specifications, the fee algorithm is very interesting. However, some issues arise due to very advanced magical witchcraft going on in this contract:
- there is no required resolver dns_resolve method in the contract, it cannot be used as contest-standartized dns resolver
- udict is used for domain category instead of idict, owner stored in category 1 instead of -2 (blocking: may prevent expected usage of resolver)
- fee calculation does not account at all for cell references: neither the magic udict_merge function does ensure that there are zero references neither it accounts their size: it is possible to put tons of data in referenced cell and drain contract balance fairly quickly
- add_domain is not resistant to dust attacks - there is no initial fee except for gas charges for domain registration
Continued in #issue8900
1
Mad Crow Nov 3, 2019 at 06:41
First of all I would say thank you for your audit, I find it really impressive. 
- Cell reference is the main issue. It is definitely necessary to prevent accepting cells with the references. - udict or idict issue is the question of standard first of all, it can be easily adjusted in future. 
- what about dns_resolve method, I strongly believe this should be performed on backend or even in the separate contract working with DNS contracts chain.
- dust attack is really possible, however I believe since reference issue is fixed, such attack is meaningless since transaction fees, those are on caller, will be significantly higher than possible storage fees.
Shiny Giraffe Nov 3, 2019 at 00:31
Continuation of #issue8899
- no gc at all unless owner asks - involves fairly crucial external dependency for contract health
- comments in the code are mess and are entirely misleading
- error replies do not have high order bit set (which is required as per smc guidelines)
1
Mad Crow Nov 3, 2019 at 04:27
Since this is the contract dealing with finances belonging to the owner of such contract, I guess the owner should handle operations and be responsible for the contract health. 
- yes, I should have learned smc guidelines carefully, however in resolve function I believe error codes have high order bit set.
- and I apologize for comments, some parts are copied from original contracts and were not cleaned properly, I was very short of time working on DNS and Payment channel, however Multisig must be much better
Shiny Giraffe Nov 3, 2019 at 07:05
Yeah, i find it respectable that you managed to work out 3 different non-related contract times in time, and multisig actually looks way more well-groomed.
Excuse me for a little bitsy of misleading, the should be dns_resolve, not dnsresolve.
Considering this, looking at all found bits about DNS resolver in TON at all, the dnsresolve is reasonably standartized resolver function that provides resolution of domain name for any particular DNS resolver in blockchain. It is noticable that resolution shall be provided by the requirements as a get function. This way, get function is executed by client, and result address should be used by it before any transaction using that result is actually sent to network. This is more supported by fact, that contract cant call get function by itself by any means atm.
[ Continued in #issue8905 ]
1
Mad Crow Nov 3, 2019 at 07:37
I see GET function as a function receiving record for desired Record ID, that is assigned to data type. So if you want to fetch subdomain, you request record of the next DNS subdomain contract by calling getter function, then get its name, the same process can be reversal if standard has assigned record id to the parent DNS contract. However from the practical side I do not see the situation when such iteration is required.
Shiny Giraffe Nov 3, 2019 at 07:05
[ Continuation of #issue8904 ]
However, this is even more important in chain of resolvers: if, for example, you want to find org\0telegram\0 entry, and root resolver tells that org\0 is delegated to your resolver, client will say "okay, now lets ask your resolver" and attempt to resolve this entry using dnsresolve(name, cat) with your resolver. This way, I see three problems here (mentioned in #issue8899): your resolver cannot perform forwarding to another chain (because there is no special -1 next entry), category #1 is clobbered by owner address (i believe that category 1 may be "primary" in future, like A or AAAA in normal DNS), and most importantly, not having standartized dnsresolve will trash the resolution when client attempts to find and call dnsresolve at your contract. This way, it should be implemented inside your contract because it is not possible to "proxy" dnsresolve through another contract (remember, no ways to call another SC get-methods).
1
Mad Crow Nov 3, 2019 at 07:39
First of all, if you check, I've implemented both getter and fetcher function. Fetcher function is extremely important. For example, you are going to create multisig wallet based on DNS, so your multisig contract send requests to DNS contracts and fetches public keys. What about resolving names such as you mentioned, backend iterates over contracts, next and previous entries can be easily specified by assigning any record index, however this should be properly standardized. As far as I got making this contract, DNS is not really standardised yet and TON is looking for suggestions how to make, so my solution is just the suggestion of the way I see this implementation not related to current DNS-resolver contract.
Clever Turkey Nov 6, 2019 at 11:40
#issue9062
It is really dangerous to not compare workchain_id, because it is very easy to register a contract in another workchain with the same address. Usually there will be no way to send messages from the contract, but that's not always the case.
1
Mad Crow Nov 6, 2019 at 11:45
I though address depends on hash of code and initial persistent data, that usually includes public key. Maybe I this is not the issue you mention and I'm getting something wrong.
Nobody added any issues yet...