[#42] add ResolveContractHash method #111

Merged
alexvanin merged 1 commits from pogpp/frostfs-sdk-go:feature/resolveContractHash into master 2023-07-13 12:40:49 +00:00
Collaborator

Signed-off-by: Pavel Pogodaev p.pogodaev@yadro.com

Signed-off-by: Pavel Pogodaev <p.pogodaev@yadro.com>
pogpp force-pushed feature/resolveContractHash from 06a3e52ac4 to bba50de5c2 2023-07-10 07:08:26 +00:00 Compare
fyrchik approved these changes 2023-07-10 09:14:50 +00:00
fyrchik left a comment
Owner

Could you also fix the commit message? (ns: Add...)

Could you also fix the commit message? (`ns: Add...`)
ns/nns.go Outdated
@ -119,0 +147,4 @@
}
strRecordValue := string(recordValue)
if err == nil {

Can it be false? We check the error 5 lines above.

Can it be false? We check the error 5 lines above.
dkirillov reviewed 2023-07-10 10:01:58 +00:00
ns/nns.go Outdated
@ -119,0 +120,4 @@
// ResolveContractHash looks up for NNS TXT records for the given container domain
// by calling `resolve` method of NNS contract. Returns the first record which represents
// valid container ID in a string format. Otherwise, returns an error.
Collaborator

It doesn't return first record which represents valid container ID in a string format

It doesn't return `first record which represents valid container ID in a string format`
dkirillov marked this conversation as resolved
dkirillov reviewed 2023-07-10 10:03:57 +00:00
ns/nns.go Outdated
@ -119,0 +125,4 @@
// ResolveContractHash MUST NOT be called before successful Dial.
//
// See also https://docs.neo.org/docs/en-us/reference/nns.html.
func (n *NNS) ResolveContractHash(domain container.Domain) (util.Uint160, error) {
Collaborator

I'm not sure if it's a good idea to use container.Domain when we resolve contract hash. Maybe it's too confusing and we should provide just string as argument. What do you think?

I'm not sure if it's a good idea to use `container.Domain` when we resolve contract hash. Maybe it's too confusing and we should provide just string as argument. What do you think?

String should be a valid domain, e.g. container.frostfs. I would rather move container.Domain structure from container package to some sort of shared package.

String should be a valid domain, e.g. `container.frostfs`. I would rather move `container.Domain` structure from container package to some sort of shared package.
dkirillov marked this conversation as resolved
pogpp force-pushed feature/resolveContractHash from bba50de5c2 to d5971e508c 2023-07-10 11:51:00 +00:00 Compare
pogpp force-pushed feature/resolveContractHash from 8e7002a4df to fd0f4c01d7 2023-07-10 11:59:43 +00:00 Compare
fyrchik requested review from fyrchik 2023-07-11 06:25:48 +00:00
aarifullin reviewed 2023-07-11 07:17:50 +00:00
ns/nns.go Outdated
@ -119,0 +147,4 @@
}
strRecordValue := string(recordValue)
if err == nil {
Collaborator

You've already checked that err != nil above :)

You've already checked that `err != nil` above :)
pogpp force-pushed feature/resolveContractHash from fd0f4c01d7 to 9338e80628 2023-07-12 19:12:25 +00:00 Compare
pogpp force-pushed feature/resolveContractHash from 9338e80628 to 693da7f400 2023-07-12 19:13:30 +00:00 Compare
pogpp force-pushed feature/resolveContractHash from 693da7f400 to b9afe7a2f9 2023-07-12 19:23:18 +00:00 Compare
dkirillov approved these changes 2023-07-13 06:38:50 +00:00
fyrchik approved these changes 2023-07-13 06:48:32 +00:00
alexvanin merged commit b9afe7a2f9 into master 2023-07-13 12:40:49 +00:00
alexvanin deleted branch feature/resolveContractHash 2023-07-13 12:40:50 +00:00
Sign in to join this conversation.
There is no content yet.