adm: Add commands to manipulate with NNS contract #981

Merged
fyrchik merged 3 commits from acid-ant/frostfs-node:feature/932-nns-register-renew into master 2024-02-28 18:57:17 +00:00
Collaborator

Close #932

In scope of this PR introduced the following commands:

  • morph nns tokens
$ frostfs-adm morph nns tokens -c cnt_create_cfg.yml -r http://morph-chain.frostfs.devenv:30333
balance.frostfs
group.frostfs
...
$ 
  • morph nns register
$ frostfs-adm morph nns register -c cnt_create_cfg.yml -r http://morph-chain.frostfs.devenv:30333 --name test.com
unable to register domain: script failed (FAULT state) due to an error: at instruction 1553 (THROW): unhandled exception: "TLD not found"
$
$ frostfs-adm morph nns register -c cnt_create_cfg.yml -r http://morph-chain.frostfs.devenv:30333 --name com
Waiting for transaction to persist...
Domain registered successfully
$ frostfs-adm morph nns register -c cnt_create_cfg.yml -r http://morph-chain.frostfs.devenv:30333 --name test.com
Waiting for transaction to persist...
Domain registered successfully
$ frostfs-adm morph nns tokens -c cnt_create_cfg.yml -r http://morph-chain.frostfs.devenv:30333 | grep test
test.com
$ 
$ 
  • morph nns renew
$ frostfs-adm morph nns renew -c cnt_create_cfg.yml -r http://morph-chain.frostfs.devenv:30333 --name test.com
Waiting for transaction to persist...
Domain renewed successfully
$ 

  • morph nns get-records
$ frostfs-adm morph nns get-records -c cnt_create_cfg.yml -r http://morph-chain.frostfs.devenv:30333 --name test.com
SOA test.com ops@frostfs.info 1707826022376 3600 600 315360000 3600
$
  • morph nns update
$ frostfs-adm morph nns update -c cnt_create_cfg.yml -r http://morph-chain.frostfs.devenv:30333 --name test.com --email "info@test.com"
Waiting for transaction to persist...
SOA records updated successfully
$ frostfs-adm morph nns get-records -c cnt_create_cfg.yml -r http://morph-chain.frostfs.devenv:30333 --name test.com
SOA test.com info@test.com 1707826188604 3600 600 315360000 3600
$
  • morph nns delete-records
$ frostfs-adm morph nns delete-records -c cnt_create_cfg.yml -r http://morph-chain.frostfs.devenv:30333 --name test.com --type TXT
Waiting for transaction to persist...
Records removed successfully
$
  • nns add-record
$ frostfs-adm morph nns add-record -c cnt_create_cfg.yml -r http://morph-chain.frostfs.devenv:30333 --name test.com --type CNAME --data "data"
Waiting for transaction to persist...
Record added successfully
$ frostfs-adm morph nns get-records -c cnt_create_cfg.yml -r http://morph-chain.frostfs.devenv:30333 --name test.com
CNAME data
SOA test.com info@test.com 1707826487027 3600 600 315360000 3600
$ 

Signed-off-by: Anton Nikiforov an.nikiforov@yadro.com

Close #932 In scope of this PR introduced the following commands: - `morph nns tokens` ``` $ frostfs-adm morph nns tokens -c cnt_create_cfg.yml -r http://morph-chain.frostfs.devenv:30333 balance.frostfs group.frostfs ... $ ``` - `morph nns register` ``` $ frostfs-adm morph nns register -c cnt_create_cfg.yml -r http://morph-chain.frostfs.devenv:30333 --name test.com unable to register domain: script failed (FAULT state) due to an error: at instruction 1553 (THROW): unhandled exception: "TLD not found" $ $ frostfs-adm morph nns register -c cnt_create_cfg.yml -r http://morph-chain.frostfs.devenv:30333 --name com Waiting for transaction to persist... Domain registered successfully $ frostfs-adm morph nns register -c cnt_create_cfg.yml -r http://morph-chain.frostfs.devenv:30333 --name test.com Waiting for transaction to persist... Domain registered successfully $ frostfs-adm morph nns tokens -c cnt_create_cfg.yml -r http://morph-chain.frostfs.devenv:30333 | grep test test.com $ $ ``` - `morph nns renew` ``` $ frostfs-adm morph nns renew -c cnt_create_cfg.yml -r http://morph-chain.frostfs.devenv:30333 --name test.com Waiting for transaction to persist... Domain renewed successfully $ ``` - `morph nns get-records` ``` $ frostfs-adm morph nns get-records -c cnt_create_cfg.yml -r http://morph-chain.frostfs.devenv:30333 --name test.com SOA test.com ops@frostfs.info 1707826022376 3600 600 315360000 3600 $ ``` - `morph nns update` ``` $ frostfs-adm morph nns update -c cnt_create_cfg.yml -r http://morph-chain.frostfs.devenv:30333 --name test.com --email "info@test.com" Waiting for transaction to persist... SOA records updated successfully $ frostfs-adm morph nns get-records -c cnt_create_cfg.yml -r http://morph-chain.frostfs.devenv:30333 --name test.com SOA test.com info@test.com 1707826188604 3600 600 315360000 3600 $ ``` - `morph nns delete-records` ``` $ frostfs-adm morph nns delete-records -c cnt_create_cfg.yml -r http://morph-chain.frostfs.devenv:30333 --name test.com --type TXT Waiting for transaction to persist... Records removed successfully $ ``` - `nns add-record` ``` $ frostfs-adm morph nns add-record -c cnt_create_cfg.yml -r http://morph-chain.frostfs.devenv:30333 --name test.com --type CNAME --data "data" Waiting for transaction to persist... Record added successfully $ frostfs-adm morph nns get-records -c cnt_create_cfg.yml -r http://morph-chain.frostfs.devenv:30333 --name test.com CNAME data SOA test.com info@test.com 1707826487027 3600 600 315360000 3600 $ ``` Signed-off-by: Anton Nikiforov <an.nikiforov@yadro.com>
acid-ant changed title from adm: Add commands to manipulate with `NNS` contract to WIP: adm: Add commands to manipulate with `NNS` contract 2024-02-13 12:02:52 +00:00
acid-ant changed title from WIP: adm: Add commands to manipulate with `NNS` contract to adm: Add commands to manipulate with `NNS` contract 2024-02-13 13:26:58 +00:00
acid-ant requested review from storage-core-committers 2024-02-13 13:27:07 +00:00
acid-ant requested review from storage-core-developers 2024-02-13 13:27:10 +00:00
dstepanov-yadro reviewed 2024-02-13 14:41:35 +00:00
@ -0,0 +19,4 @@
addRecordCmd.Flags().String(commonflags.AlphabetWalletsFlag, "", commonflags.AlphabetWalletsFlagDesc)
addRecordCmd.Flags().String(commonflags.NNSNameFlag, "", commonflags.NNSNameFlagDesc)
addRecordCmd.Flags().String(commonflags.NNSRecordTypeFlag, "", commonflags.NNSRecordTypeFlagDesc)
addRecordCmd.Flags().String(commonflags.NNSRecordDataFlag, "", "Domain name service record data")

Why "Domain name service record data" is not in constants?

Why `"Domain name service record data"` is not in constants?
Poster
Collaborator

Thanks, moved to constant.

Thanks, moved to constant.
dstepanov-yadro marked this conversation as resolved
@ -0,0 +114,4 @@
}
func getRecordType(recordType string) (*big.Int, error) {
switch strings.ToUpper(recordType) {

AAAA?

`AAAA`?
Poster
Collaborator

Ooops, fixed.

Ooops, fixed.
dstepanov-yadro marked this conversation as resolved
acid-ant force-pushed feature/932-nns-register-renew from 5b3f315a08 to d327ee61da 2024-02-14 06:00:17 +00:00 Compare
fyrchik reviewed 2024-02-14 06:10:17 +00:00
@ -39,0 +46,4 @@
NNSRecordTypeFlag = "type"
NNSRecordTypeFlagDesc = "Domain name service record type(A|CNAME|SOA|TXT)"
NNSRecordDataFlag = "data"
NNSRecordDataFlagDesc = "Domain name service record data"

Why are this flags in commonflags package and not in NNS?

Why are this flags in `commonflags` package and not in NNS?
Poster
Collaborator

Moved to nns and reduced visibility.

Moved to `nns` and reduced visibility.
fyrchik marked this conversation as resolved
@ -0,0 +16,4 @@
updateCmd.Flags().String(commonflags.NNSNameFlag, "", commonflags.NNSNameFlagDesc)
updateCmd.Flags().String(commonflags.NNSEmailFlag, constants.FrostfsOpsEmail, "Domain owner email")
updateCmd.Flags().Int64(commonflags.NNSRefreshFlag, constants.NNSRefreshDefVal,
"The number of seconds between update requests from secondary and slave name servers")

We are not writing a DNS server here, I wonder, whether this description is of any help to anyone. Why not just refer to "SOA record REFRESH parameter"

We are not writing a DNS server here, I wonder, whether this description is of any help to anyone. Why not just refer to "SOA record REFRESH parameter"
Poster
Collaborator

Agree, made description shorter as you proposed.

Agree, made description shorter as you proposed.
fyrchik marked this conversation as resolved
acid-ant force-pushed feature/932-nns-register-renew from d327ee61da to 7010ec6d72 2024-02-14 06:30:58 +00:00 Compare
fyrchik reviewed 2024-02-14 07:57:46 +00:00
@ -36,4 +36,5 @@ const (
HomomorphicHashDisabledInitFlag = "network.homomorphic_hash_disabled"
CustomZoneFlag = "domain"
AlphabetSizeFlag = "size"
NNSNameFlag = "name"

And this one?

And this one?
Poster
Collaborator

Shame on me. Fixed.

Shame on me. Fixed.
fyrchik marked this conversation as resolved
acid-ant force-pushed feature/932-nns-register-renew from 7010ec6d72 to f9249160dd 2024-02-14 08:01:48 +00:00 Compare
acid-ant force-pushed feature/932-nns-register-renew from f9249160dd to 27942c547b 2024-02-14 08:03:26 +00:00 Compare
fyrchik approved these changes 2024-02-14 08:41:34 +00:00
fyrchik left a comment
Owner
  1. What testing was done?
  2. Have you checked testlib/testcases code?
    Also, ask CI guys, they could use adm in pipelines.
1. What testing was done? 2. Have you checked testlib/testcases code? Also, ask CI guys, they could use adm in pipelines.
dstepanov-yadro approved these changes 2024-02-14 08:43:10 +00:00
Poster
Collaborator
  1. What testing was done?

Manual testing and checking for receiving errors from adm and nns contract.

  1. Have you checked testlib/testcases code?
    Also, ask CI guys, they could use adm in pipelines.

QA team informed about these changes.

> 1. What testing was done? Manual testing and checking for receiving errors from `adm` and `nns` contract. > 2. Have you checked testlib/testcases code? > Also, ask CI guys, they could use adm in pipelines. QA team informed about these changes.
fyrchik requested changes 2024-02-14 09:42:08 +00:00
fyrchik left a comment
Owner

This will have the same problems as policy contract: it works fine on dev-env with 1 committee node, but will fail in multi-committee scenario.
Can we make sure it doesn't fail in this scenario before merge?

This will have the same problems as policy contract: it works fine on dev-env with 1 committee node, but will fail in multi-committee scenario. Can we make sure it doesn't fail in this scenario before merge?
acid-ant force-pushed feature/932-nns-register-renew from 27942c547b to 4773f9aedd 2024-02-20 08:37:20 +00:00 Compare
Poster
Collaborator

This will have the same problems as policy contract: it works fine on dev-env with 1 committee node, but will fail in multi-committee scenario.
Can we make sure it doesn't fail in this scenario before merge?

Added custom Actor. Tested successfully with dev-env and virtual environment.

> This will have the same problems as policy contract: it works fine on dev-env with 1 committee node, but will fail in multi-committee scenario. > Can we make sure it doesn't fail in this scenario before merge? Added custom `Actor`. Tested successfully with dev-env and virtual environment.
acid-ant requested review from dstepanov-yadro 2024-02-20 08:38:59 +00:00
acid-ant requested review from fyrchik 2024-02-20 08:39:02 +00:00
acid-ant changed title from adm: Add commands to manipulate with `NNS` contract to WIP: adm: Add commands to manipulate with `NNS` contract 2024-02-20 08:53:38 +00:00
acid-ant changed title from WIP: adm: Add commands to manipulate with `NNS` contract to adm: Add commands to manipulate with `NNS` contract 2024-02-20 08:56:44 +00:00
dstepanov-yadro reviewed 2024-02-20 09:58:44 +00:00
@ -0,0 +83,4 @@
// Inside the methods `MakeCall` and `SendRun` of the NeoGO's actor transaction is signing by committee account,
// because actor uses committee wallet.
// But it is not enough, need to sign with another committee accounts.
func (a *MorphActor) sign(tx *transaction.Transaction) error {

Maybe rename sign -> resign ?

Maybe rename `sign` -> `resign` ?
Poster
Collaborator

Renamed.

Renamed.
dstepanov-yadro approved these changes 2024-02-20 09:59:42 +00:00
acid-ant force-pushed feature/932-nns-register-renew from 4773f9aedd to efa30cb2e4 2024-02-20 10:13:58 +00:00 Compare
fyrchik reviewed 2024-02-20 15:47:40 +00:00
@ -0,0 +83,4 @@
// Inside the methods `MakeCall` and `SendRun` of the NeoGO's actor transaction is signing by committee account,
// because actor uses committee wallet.
// But it is not enough, need to sign with another committee accounts.
func (a *MorphActor) resign(tx *transaction.Transaction) error {

It is a kludge (why sign in the first place), but probably worth because the amount of code to copy was too high?

Anyway, this makes our MakeRun and MakeCall actually invalid if they are used from the outside e.g. in auto-generated bindings. What about making them panic to ensure we do not accidentally use them?

It is a kludge (why sign in the first place), but probably worth because the amount of code to copy was too high? Anyway, this makes our `MakeRun` and `MakeCall` actually invalid if they are used from the outside e.g. in auto-generated bindings. What about making them `panic` to ensure we do not accidentally use them?
Poster
Collaborator

Make sense, added panic for unused methods.

Make sense, added `panic` for unused methods.
fyrchik marked this conversation as resolved
acid-ant force-pushed feature/932-nns-register-renew from efa30cb2e4 to e0d524e419 2024-02-20 16:14:18 +00:00 Compare
acid-ant force-pushed feature/932-nns-register-renew from e0d524e419 to 0d11826a16 2024-02-20 16:14:40 +00:00 Compare
acid-ant force-pushed feature/932-nns-register-renew from 0d11826a16 to da2be98406 2024-02-22 12:21:11 +00:00 Compare
acid-ant force-pushed feature/932-nns-register-renew from da2be98406 to 9c5cfd56b3 2024-02-26 06:34:58 +00:00 Compare
fyrchik approved these changes 2024-02-26 08:16:19 +00:00
@ -0,0 +21,4 @@
"github.com/spf13/viper"
)
const errUnimplemented string = "unimplemented"

type is unnecessary, also "not implemented" is somewhat common, not sure we need constant here, like even in Go compiler:

$ rg 'panic\("bad index"\)'
src/cmd/internal/obj/sym.go
264:                                    panic("bad index")
272:                                    panic("bad index")
281:                            panic("bad index")
289:                            panic("bad index")
324:                            panic("bad index")
type is unnecessary, also "not implemented" is somewhat common, not sure we need constant here, like even in Go compiler: ``` $ rg 'panic\("bad index"\)' src/cmd/internal/obj/sym.go 264: panic("bad index") 272: panic("bad index") 281: panic("bad index") 289: panic("bad index") 324: panic("bad index") ```
Poster
Collaborator

Agree, updated.

Agree, updated.
@ -0,0 +23,4 @@
const errUnimplemented string = "unimplemented"
// MorphActor is a kludge, do not use it outside of the morph commands.

More like LocalAuthor?

More like `LocalAuthor`?
Poster
Collaborator

Renamed.

Renamed.
aarifullin reviewed 2024-02-26 09:21:07 +00:00
@ -0,0 +83,4 @@
recordTypeToString(nns.RecordType(rs[1].Value().(*big.Int).Int64())),
string(bs))
}
items, err = act.Invoker.TraverseIterator(sid, &r, 0)
Collaborator

Here is no check for err if TraverseIterator was not successful

Here is no check for `err` if `TraverseIterator` was not successful
Poster
Collaborator

Thanks, missed that. Updated.

Thanks, missed that. Updated.
aarifullin marked this conversation as resolved
fyrchik reviewed 2024-02-26 16:37:54 +00:00
@ -0,0 +64,4 @@
}
func getRecords(cmd *cobra.Command, _ []string) {
c, act, hash := getRPCClient(cmd)

addRecord and getRecords use the same function to receive RPC client
Won't we run in the same issue as #1009, namely reading alphabet wallets for methods which only read and do not sign anything?

`addRecord` and `getRecords` use the same function to receive RPC client Won't we run in the same issue as #1009, namely reading alphabet wallets for methods which only read and do not sign anything?
Poster
Collaborator

Good catch. Added ability for read operations (tokens, get-records) execute without alphabet wallets.

Good catch. Added ability for read operations (`tokens`, `get-records`) execute without alphabet wallets.
acid-ant force-pushed feature/932-nns-register-renew from 9c5cfd56b3 to 386bac6d4c 2024-02-26 16:45:12 +00:00 Compare
acid-ant force-pushed feature/932-nns-register-renew from 386bac6d4c to 0f6857c698 2024-02-26 17:15:07 +00:00 Compare
acid-ant force-pushed feature/932-nns-register-renew from 0f6857c698 to a8709d54bd 2024-02-26 18:11:51 +00:00 Compare
aarifullin approved these changes 2024-02-27 09:49:04 +00:00
aarifullin left a comment
Collaborator

LGTM

LGTM
fyrchik approved these changes 2024-02-28 18:57:06 +00:00
fyrchik merged commit bc9dbb26ec into master 2024-02-28 18:57:17 +00:00
Sign in to join this conversation.
No Milestone
No Assignees
4 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: TrueCloudLab/frostfs-node#981
There is no content yet.