Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide UnmarshalJSON() for common.uint160 #944

Merged
merged 1 commit into from
Aug 2, 2023
Merged

Conversation

omani
Copy link
Contributor

@omani omani commented Aug 1, 2023

fixes error json: cannot unmarshal string into Go struct field Account.ProgramHash of type common.Uint160 in case nkn.Account struct is to be unmarshalled.

common/uint160.go Outdated Show resolved Hide resolved
common/uint160.go Outdated Show resolved Hide resolved
@omani omani force-pushed the master branch 2 times, most recently from f4e029b to 10773eb Compare August 2, 2023 02:01
if len(in) < 2 {
return errors.New("[Common]: Uint160UnmarshalJSON err, len < 2")
}
f, err = ToScriptHash(string(in[1 : len(in)-1]))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will not update f. You need to change
func (f Uint160)
to
func (f *Uint160)
then change f, err = to temp, err :=,
and finally call f.SetBytes(temp.ToArray()) to update f.

@yilunzhang
Copy link
Member

yilunzhang commented Aug 2, 2023

FYI I asked ChatGPT to write a unit test for this. This is the prompt I used:

Write unit tests for the following json marshal/unmarshal implementations:

 func (f Uint160) MarshalJSON() ([]byte, error) {
	str, err := f.ToAddress()
 	return []byte("\"" + str + "\""), err
 }

 func (f Uint160) UnmarshalJSON(in []byte) (err error) {
 	if len(in) < 2 {
 		return errors.New("[Common]: Uint160UnmarshalJSON err, len < 2")
 	}
 	f, err = ToScriptHash(string(in[1 : len(in)-1]))
 	return
 }

Results I got:

package yourpackage

import (
	"testing"
	"errors"
)

func TestMarshalJSON(t *testing.T) {
	// Create instance of Uint160
	f := new(Uint160)
	
	// Expected value after Marshalling
	expected := []byte("\"expectedValue\"")

	bytes, err := f.MarshalJSON()
	if err != nil {
		t.Fatalf("Unexpected error: %v", err)
	}

	if !bytesEqual(bytes, expected) {
		t.Fatalf("Expected %v, got %v", expected, bytes)
	}

	// TODO: If possible, create a scenario where ToAddress returns an error
	// and verify that MarshalJSON also returns this error.
}

func TestUnmarshalJSON(t *testing.T) {
	// Input for unmarshalling
	input := []byte("\"validInput\"")

	f := new(Uint160)
	err := f.UnmarshalJSON(input)
	if err != nil {
		t.Fatalf("Unexpected error: %v", err)
	}

	// TODO: Verify that f has been updated correctly

	input = []byte("\"a\"")
	err = f.UnmarshalJSON(input)
	if err == nil || err.Error() != "[Common]: Uint160UnmarshalJSON err, len < 2" {
		t.Fatalf("Expected error, got: %v", err)
	}

	// TODO: If possible, create a scenario where ToScriptHash returns an error
	// and verify that UnmarshalJSON also returns this error.
}

func bytesEqual(a, b []byte) bool {
	if len(a) != len(b) {
		return false
	}
	for i, v := range a {
		if v != b[i] {
			return false
		}
	}
	return true
}

fixes error `json: cannot unmarshal string into Go struct field
Account.ProgramHash of type common.Uint160` in case `nkn.Account` struct
is to be unmarshalled.

Signed-off-by: omani <3346207+omani@users.noreply.github.com>
@omani
Copy link
Contributor Author

omani commented Aug 2, 2023

Ive included the unit tests.

regarding...

// TODO: If possible, create a scenario where ToAddress returns an error
// and verify that MarshalJSON also returns this error.

I dont know how to do this since SetBytes() prevents us from ever having a wrong address. it will get filled with zeros and a malformed version will always result in address NKNBV71h5oUBjknMHPikPU7nraw9KtksWjtB with no error.

@yilunzhang
Copy link
Member

I dont know how to do this since SetBytes() prevents us from ever having a wrong address. it will get filled with zeros and a malformed version will always result in address NKNBV71h5oUBjknMHPikPU7nraw9KtksWjtB with no error.

Don't worry, the comments are auto generated by ChatGPT too, not all of them apply to our case.

Copy link
Member

@yilunzhang yilunzhang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

@yilunzhang yilunzhang merged commit de5a1bc into nknorg:master Aug 2, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants