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.

Sign up for GitHub

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

Jump to bottom

feat: foo20 check an error from grc20 package #977

Merged
merged 2 commits into from
Jul 20, 2023
Merged

feat: foo20 check an error from grc20 package #977

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dfb2bed
feat: foo20 check an error from grc20 package
r3v4s Jul 17, 2023
8340928
style: remove a2u helper
r3v4s Jul 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 24 additions & 6 deletions examples/gno.land/r/demo/foo20/foo20.gno
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,17 +49,26 @@ func Allowance(owner, spender users.AddressOrName) uint64 {

func Transfer(to users.AddressOrName, amount uint64) {
caller := std.PrevRealm().Addr()
foo.Transfer(caller, to.Resolve(), amount)
err := foo.Transfer(caller, to.Resolve(), amount)
if err != nil {
panic(err)
}
}

func Approve(spender users.AddressOrName, amount uint64) {
caller := std.PrevRealm().Addr()
foo.Approve(caller, spender.Resolve(), amount)
err := foo.Approve(caller, spender.Resolve(), amount)
if err != nil {
panic(err)
}
}

func TransferFrom(from, to users.AddressOrName, amount uint64) {
caller := std.PrevRealm().Addr()
foo.TransferFrom(caller, from.Resolve(), to.Resolve(), amount)
err := foo.TransferFrom(caller, from.Resolve(), to.Resolve(), amount)
if err != nil {
panic(err)
}
}

// faucet.
Expand All @@ -68,21 +77,30 @@ func Faucet() {
// FIXME: add limits?
// FIXME: add payment in gnot?
caller := std.PrevRealm().Addr()
foo.Mint(caller, 1000*10000) // 1k
err := foo.Mint(caller, 1000*10000) // 1k
if err != nil {
panic(err)
}
}

// administration.

func Mint(address users.AddressOrName, amount uint64) {
caller := std.PrevRealm().Addr()
assertIsAdmin(caller)
foo.Mint(address.Resolve(), amount)
err := foo.Mint(address.Resolve(), amount)
if err != nil {
panic(err)
}
}

func Burn(address users.AddressOrName, amount uint64) {
caller := std.PrevRealm().Addr()
assertIsAdmin(caller)
foo.Burn(address.Resolve(), amount)
err := foo.Burn(address.Resolve(), amount)
if err != nil {
panic(err)
}
}

// render.
Expand Down
40 changes: 40 additions & 0 deletions examples/gno.land/r/demo/foo20/foo20_test.gno
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,3 +54,43 @@ func TestReadOnlyPublicMethods(t *testing.T) {
}
}
}

func TestErrConditions(t *testing.T) {
admin := std.Address("g1us8428u2a5satrlxzagqqa5m6vmuze025anjlj")
empty := std.Address("")
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add another case?

	invalid := std.Address("invalid")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

grc20 package uses checkIsValidAddress to check whether address is valid or not.

gno/examples/gno.land/p/demo/grc/grc20/util.gno

Lines 7 to 12 in 973be98

func checkIsValidAddress(addr std.Address) error {
if addr.String() == "" {
return ErrInvalidAddress
}
return nil
}

unfortunately it only checks addr.String() == "", so that invalid won't be invalid but valid.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

perhaps better to rename above function to checkIsEmptyAddress

Copy link
Contributor

@waymobetta waymobetta Jul 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Ethereum ecosystem libs like Ethers and web3.js commonly utilize the isAddress terminology, if the aim is for close parity or to minimize confusion errors for devs.

SO answer with additional context.

Copy link
Member

Choose a reason for hiding this comment

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

Let's handle this in the other refactor #952.

In this particular case, using isAddress doesn't seem suitable because we're dealing with an std.Address object and checking its validity.

Creating a more general method like std.Address{}.IsValid() would be more appropriate in the future. For now, I'll focus on improving the check in the other PR or add a comment for clarification.

Thank you all for your suggestions.


type test struct {
name string
msg string
fn func()
}

std.TestSetOrigCaller(admin)
{
tests := []test{
{"Transfer(admin, 1)", "cannot send transfer to self", func() { Transfer(a2u(admin), 1) }},
{"Approve(empty, 1))", "invalid address", func() { Approve(a2u(empty), 1) }},
}
for _, tc := range tests {
shouldPanicWithMsg(t, tc.fn, tc.msg)
}
}
}

func a2u(addr std.Address) users.AddressOrName {
return users.AddressOrName(addr)
}
r3v4s marked this conversation as resolved.
Show resolved Hide resolved

func shouldPanicWithMsg(t *testing.T, f func(), msg string) {
defer func() {
if r := recover(); r == nil {
t.Errorf("The code did not panic")
} else {
errMsg := error(r).Error()
if errMsg != msg {
t.Errorf("excepted panic(%v), got(%v)", msg, errMsg)
}
}
}()
f()
}