Fix/Separate deploying contract and NNS updating transaction #1683
Conversation
carpawell
added
enhancement
Codecov Report
@@ Coverage Diff @@
## master #1683 +/- ##
==========================================
- Coverage 33.21% 32.55% -0.67%
==========================================
Files 332 337 +5
Lines 22752 22667 -85
==========================================
- Hits 7558 7380 -178
- Misses 14574 14674 +100
+ Partials 620 613 -7
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
NNS has |
|
I didn't like the idea of changing NNS pricing in general for all. I have nothing against temporary NNS price change, contract deploy and setting price back in the same Tx. This doesn't mean we don't have to check deploy actions sanity first =) |
|
Yes, the whole purpose of the optimisation is to do it temporarily, so that there are no visible side-effects for other transactions. |
|
@fyrchik, added hack with changing price but kept separate deploying and registering. Also, added |
| } | ||
|
|
||
| // add record to NNS | ||
| w = io.NewBufBinWriter() |
There was a problem hiding this comment.
BufBinWriter can be reused after previous transaction. Use w.Reset() to reset it.
| "`%s` does not equal NNS's `%s`; use `--%s` flag to deploy with that key anyway", | ||
| contractWalletFilename, morphclient.NNSGroupKeyName, forceFlag, |
There was a problem hiding this comment.
Maybe some %s doesn't contain NNS's group key... will be better? The first argument is wallet filename, this message looks strange a bit.
There was a problem hiding this comment.
decided not to deploy custom contracts with group.neofs group, removed that code
| if err != nil { | ||
| return fmt.Errorf("can't sign manifest group: %v", err) | ||
| } | ||
|
|
There was a problem hiding this comment.
forgot that the linter already works and requires attention
|
Removed |
|
@carpawell why did you separate deploy and register? |
@fyrchik that was the original idea of the PR. Yes, now |
|
Yes, the less transactions we have, the fewer places where something could go wrong. |
Register NNS domain in one TX: 1. Set minimal (`1`) registration price; 2. Register domain; 3. Return registration price back. Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
|
@fyrchik, ok, dropped that commit. |
Register NNS domain in one TX: 1. Set minimal (`1`) registration price; 2. Register domain; 3. Return registration price back. Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
Deploy contract in one transaction and register NNS record in another to
prevent GAS limit exceeding due to the fact that registration always takes
10+ GAS.
Signed-off-by: Pavel Karpy carpawell@nspcc.ru
Closes #1675.