Skip to content

Commit

Permalink
feat: full std.Address validation (#1799)
Browse files Browse the repository at this point in the history
Length of `std.Address` was only condition to check whether address is
valid or not.
Fixing it by checking whether address can be decoded

<details><summary>Contributors' checklist...</summary>

- [x] Added new tests, or not needed, or not feasible
- [ ] Provided an example (e.g. screenshot) to aid review or the PR is
self-explanatory
- [x] Updated the official documentation or not needed
- [x] No breaking changes were made, or a `BREAKING CHANGE: xxx` message
was included in the description
- [ ] Added references to related issues and PRs
- [ ] Provided any useful hints for running manual tests
- [ ] Added new benchmarks to [generated
graphs](https://gnoland.github.io/benchmarks), if any. More info
[here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md).
</details>
  • Loading branch information
r3v4s authored Mar 28, 2024
1 parent 8376831 commit cb52ae7
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 13 deletions.
19 changes: 10 additions & 9 deletions examples/gno.land/r/demo/foo20/foo20_test.gno
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,14 @@ import (
"std"
"testing"

"gno.land/p/demo/users"
pusers "gno.land/p/demo/users"
"gno.land/r/demo/users"
)

func TestReadOnlyPublicMethods(t *testing.T) {
admin := users.AddressOrName("g1us8428u2a5satrlxzagqqa5m6vmuze025anjlj")
manfred := users.AddressOrName("g1u7y667z64x2h7vc6fmpcprgey4ck233jaww9zq")
unknown := std.Address("g1u0000000000000000000000000000000000000")
admin := pusers.AddressOrName("g1us8428u2a5satrlxzagqqa5m6vmuze025anjlj")
manfred := pusers.AddressOrName("g1u7y667z64x2h7vc6fmpcprgey4ck233jaww9zq")
unknown := pusers.AddressOrName("g1jg8mtutu9khhfwc4nxmuhcpftf0pajdhfvsqf5") // valid but never used.

type test struct {
name string
Expand All @@ -25,7 +26,7 @@ func TestReadOnlyPublicMethods(t *testing.T) {
{"BalanceOf(admin)", 10000000000, func() uint64 { return BalanceOf(admin) }},
{"BalanceOf(manfred)", 100000000, func() uint64 { return BalanceOf(manfred) }},
{"Allowance(admin, manfred)", 0, func() uint64 { return Allowance(admin, manfred) }},
{"BalanceOf(unknown)", 0, func() uint64 { return BalanceOf(users.AddressOrName(unknown)) }},
{"BalanceOf(unknown)", 0, func() uint64 { return BalanceOf(unknown) }},
}
for _, tc := range tests {
if tc.fn() != tc.balance {
Expand All @@ -35,7 +36,7 @@ func TestReadOnlyPublicMethods(t *testing.T) {
}

// unknown uses the faucet.
std.TestSetOrigCaller(unknown)
std.TestSetOrigCaller(users.Resolve(unknown))
Faucet()

// check balances #2.
Expand All @@ -45,7 +46,7 @@ func TestReadOnlyPublicMethods(t *testing.T) {
{"BalanceOf(admin)", 10000000000, func() uint64 { return BalanceOf(admin) }},
{"BalanceOf(manfred)", 100000000, func() uint64 { return BalanceOf(manfred) }},
{"Allowance(admin, manfred)", 0, func() uint64 { return Allowance(admin, manfred) }},
{"BalanceOf(unknown)", 10000000, func() uint64 { return BalanceOf(users.AddressOrName(unknown)) }},
{"BalanceOf(unknown)", 10000000, func() uint64 { return BalanceOf(unknown) }},
}
for _, tc := range tests {
if tc.fn() != tc.balance {
Expand All @@ -68,8 +69,8 @@ func TestErrConditions(t *testing.T) {
std.TestSetOrigCaller(admin)
{
tests := []test{
{"Transfer(admin, 1)", "cannot send transfer to self", func() { Transfer(users.AddressOrName(admin), 1) }},
{"Approve(empty, 1))", "invalid address", func() { Approve(users.AddressOrName(empty), 1) }},
{"Transfer(admin, 1)", "cannot send transfer to self", func() { Transfer(pusers.AddressOrName(admin), 1) }},
{"Approve(empty, 1))", "invalid address", func() { Approve(pusers.AddressOrName(empty), 1) }},
}
for _, tc := range tests {
shouldPanicWithMsg(t, tc.fn, tc.msg)
Expand Down
12 changes: 12 additions & 0 deletions gno.land/cmd/gnoland/testdata/grc20-invalid-address.txtar
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# Test for https://github.com/gnolang/gno/pull/1799
loadpkg gno.land/r/demo/foo20

gnoland start

# execute Faucet
gnokey maketx call -pkgpath gno.land/r/demo/foo20 -func Faucet -gas-fee 1000000ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test test1
stdout 'OK!'

# execute Transfer for invalid address
! gnokey maketx call -pkgpath gno.land/r/demo/foo20 -func Transfer -args g1ubwj0apf60hd90txhnh855fkac34rxlsvua0aa -args 1 -gas-fee 1000000ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test test1
stderr '"gnokey" error: --= Error =--\nData: invalid address'
5 changes: 3 additions & 2 deletions gnovm/stdlibs/std/crypto.gno
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@ func (a Address) String() string {
return string(a)
}

// IsValid checks if the address is of specific length. Doesn't check prefix or checksum for the address
// IsValid checks if the address is valid bech32 encoded string
func (a Address) IsValid() bool {
return len(a) == RawAddressSize*2 // hex length
_, _, ok := DecodeBech32(a)
return ok
}

const RawAddressSize = 20
Expand Down
11 changes: 11 additions & 0 deletions gnovm/stdlibs/std/crypto_test.gno
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,17 @@ func TestValid(t *testing.T) {
{inputAddress: "g127jydsh6cms3lrtdenydxsckh23a8d6emqcvfa", expected: true},
{inputAddress: "g1u7y667z64x2h7vc6fmpcprgey4ck233jaww9zq", expected: true},
{inputAddress: "g14da4n9hcynyzz83q607uu8keuh9hwlv42ra6fa", expected: true},

// Bech32 doesn't allow '1', 'b', 'i', 'o' for data part
//
// https://github.com/bitcoin/bips/blob/master/bip-0173.mediawiki#bech32
{inputAddress: "g1jg8mtutu9khhfwc4nxmuhcpftf0pajdhfvsqf5", expected: true},
{inputAddress: "g1jg8mtutu9khhfwc4nxmuhcpftf0pajdhfvsqf1", expected: false},
{inputAddress: "g1jg8mtutu9khhfwc4nxmuhcpftf0pajdhfvsqfb", expected: false},
{inputAddress: "g1jg8mtutu9khhfwc4nxmuhcpftf0pajdhfvsqfi", expected: false},
{inputAddress: "g1jg8mtutu9khhfwc4nxmuhcpftf0pajdhfvsqfo", expected: false},

{inputAddress: "g1u0000000000000000000000000000000000000", expected: false},
{inputAddress: "", expected: false},
{inputAddress: "000000000000", expected: false},
{inputAddress: "0000000000000000000000000000000000000000000000000000000000000000000000", expected: false},
Expand Down
5 changes: 3 additions & 2 deletions gnovm/stdlibs/stdshim/crypto.gno
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@ func (a Address) String() string {
return string(a)
}

// IsValid checks if the address is of specific length. Doesn't check prefix or checksum for the address
// IsValid checks if the address is valid bech32 encoded string
func (a Address) IsValid() bool {
return len(a) == RawAddressSize*2 // hex length
_, _, ok := DecodeBech32(a)
return ok
}

const RawAddressSize = 20
Expand Down

0 comments on commit cb52ae7

Please sign in to comment.