From 2e6f18a8f9da8ab847ffbd184079a9b2b3d4b844 Mon Sep 17 00:00:00 2001 From: Blake <104744707+r3v4s@users.noreply.github.com> Date: Wed, 3 Jul 2024 00:46:58 +0900 Subject: [PATCH] feat(stdlibs/std): restrict Banker methods based on caller of GetBanker (#1921) related pr #1787 There was bit of extra conversion in previous pr after merged. 1) revert test cases 2) allow `Send` from realm that created banker rather the one calling
Contributors' checklist... - [x] Added new tests, or not needed, or not feasible - [x] 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 - [x] 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).
--------- Co-authored-by: Morgan --- docs/concepts/stdlibs/coin.md | 2 +- .../gno.land/r/demo/banktest/banktest.gno | 2 +- .../gno.land/r/demo/banktest/z_0_filetest.gno | 35 ++++++++++--------- .../gno.land/r/demo/banktest/z_1_filetest.gno | 1 + .../gno.land/r/demo/banktest/z_3_filetest.gno | 20 +++++++++++ .../gno.land/r/demo/wugnot/z0_filetest.gno | 32 +++++++---------- gnovm/stdlibs/std/banker.gno | 26 ++++++++++++-- gnovm/stdlibs/std/banker.go | 17 --------- 8 files changed, 77 insertions(+), 58 deletions(-) create mode 100644 examples/gno.land/r/demo/banktest/z_3_filetest.gno diff --git a/docs/concepts/stdlibs/coin.md b/docs/concepts/stdlibs/coin.md index 46c7c519f7c..50a0e65829d 100644 --- a/docs/concepts/stdlibs/coin.md +++ b/docs/concepts/stdlibs/coin.md @@ -32,4 +32,4 @@ which can manipulate them depending on access rights. Read more about coins in the [Effective Gno](../effective-gno.md#coins) section. -The Coin(s) API can be found in under the `std` package [reference](../../reference/stdlibs/std/coin.md). +The Coin(s) API can be found in under the `std` package [reference](../../reference/stdlibs/std/coin.md). \ No newline at end of file diff --git a/examples/gno.land/r/demo/banktest/banktest.gno b/examples/gno.land/r/demo/banktest/banktest.gno index 1a23838dcfc..29c479dd025 100644 --- a/examples/gno.land/r/demo/banktest/banktest.gno +++ b/examples/gno.land/r/demo/banktest/banktest.gno @@ -40,7 +40,7 @@ func Deposit(returnDenom string, returnAmount int64) string { // return if any. if returnAmount > 0 { banker := std.GetBanker(std.BankerTypeOrigSend) - pkgaddr := std.CurrentRealm().Addr() + pkgaddr := std.GetOrigPkgAddr() // TODO: use std.Coins constructors, this isn't generally safe. banker.SendCoins(pkgaddr, caller, send) return "returned!" diff --git a/examples/gno.land/r/demo/banktest/z_0_filetest.gno b/examples/gno.land/r/demo/banktest/z_0_filetest.gno index c997d92c53a..4ea76bbe17a 100644 --- a/examples/gno.land/r/demo/banktest/z_0_filetest.gno +++ b/examples/gno.land/r/demo/banktest/z_0_filetest.gno @@ -1,8 +1,7 @@ -package main - -// NOTE: this doesn't do anything, as it sends to "main". // SEND: 100000000ugnot +package main + import ( "std" @@ -10,38 +9,40 @@ import ( ) func main() { + // set up main address and banktest addr. banktestAddr := std.DerivePkgAddr("gno.land/r/demo/banktest") - - // print main balance before. mainaddr := std.DerivePkgAddr("main") std.TestSetOrigCaller(mainaddr) + std.TestSetOrigPkgAddr(banktestAddr) - banker := std.GetBanker(std.BankerTypeReadonly) + // get and print balance of mainaddr. + // with the SEND, + 200 gnot given by the TestContext, main should have 300gnot. + banker := std.GetBanker(std.BankerTypeRealmSend) mainbal := banker.GetCoins(mainaddr) - println("main before:", mainbal) // plus OrigSend equals 300. + println("main before:", mainbal) - // simulate a Deposit call. - std.TestIssueCoins(banktestAddr, std.Coins{{"ugnot", 100000000}}) - std.TestSetOrigSend(std.Coins{{"ugnot", 100000000}}, nil) - res := banktest.Deposit("ugnot", 100000000) + // simulate a Deposit call. use Send + OrigSend to simulate -send. + banker.SendCoins(mainaddr, banktestAddr, std.Coins{{"ugnot", 100_000_000}}) + std.TestSetOrigSend(std.Coins{{"ugnot", 100_000_000}}, nil) + res := banktest.Deposit("ugnot", 50_000_000) println("Deposit():", res) // print main balance after. mainbal = banker.GetCoins(mainaddr) - println("main after:", mainbal) // still 300. + println("main after:", mainbal) - // simulate a Render(). + // simulate a Render(). banker should have given back all coins. res = banktest.Render("") println(res) } // Output: -// main before: 200000000ugnot +// main before: 300000000ugnot // Deposit(): returned! -// main after: 300000000ugnot +// main after: 250000000ugnot // ## recent activity // -// * g17rgsdnfxzza0sdfsdma37sdwxagsz378833ca4 100000000ugnot sent, 100000000ugnot returned, at 2009-02-13 11:31pm UTC +// * g17rgsdnfxzza0sdfsdma37sdwxagsz378833ca4 100000000ugnot sent, 50000000ugnot returned, at 2009-02-13 11:31pm UTC // // ## total deposits -// 300000000ugnot +// 50000000ugnot diff --git a/examples/gno.land/r/demo/banktest/z_1_filetest.gno b/examples/gno.land/r/demo/banktest/z_1_filetest.gno index 3c25a148c35..8f9f7647036 100644 --- a/examples/gno.land/r/demo/banktest/z_1_filetest.gno +++ b/examples/gno.land/r/demo/banktest/z_1_filetest.gno @@ -10,6 +10,7 @@ func main() { banktestAddr := std.DerivePkgAddr("gno.land/r/demo/banktest") // simulate a Deposit call. + std.TestSetOrigPkgAddr(banktestAddr) std.TestIssueCoins(banktestAddr, std.Coins{{"ugnot", 100000000}}) std.TestSetOrigSend(std.Coins{{"ugnot", 100000000}}, nil) res := banktest.Deposit("ugnot", 101000000) diff --git a/examples/gno.land/r/demo/banktest/z_3_filetest.gno b/examples/gno.land/r/demo/banktest/z_3_filetest.gno new file mode 100644 index 00000000000..ca8717dfcc9 --- /dev/null +++ b/examples/gno.land/r/demo/banktest/z_3_filetest.gno @@ -0,0 +1,20 @@ +package main + +import ( + "std" +) + +func main() { + banktestAddr := std.DerivePkgAddr("gno.land/r/demo/banktest") + + mainaddr := std.DerivePkgAddr("main") + std.TestSetOrigCaller(mainaddr) + + banker := std.GetBanker(std.BankerTypeRealmSend) + send := std.Coins{{"ugnot", 123}} + banker.SendCoins(banktestAddr, mainaddr, send) + +} + +// Error: +// can only send coins from realm that created banker "g17rgsdnfxzza0sdfsdma37sdwxagsz378833ca4", not "g1dv3435088tlrgggf745kaud0ptrkc9v42k8llz" diff --git a/examples/gno.land/r/demo/wugnot/z0_filetest.gno b/examples/gno.land/r/demo/wugnot/z0_filetest.gno index 6eb47d7636d..bef65c03b68 100644 --- a/examples/gno.land/r/demo/wugnot/z0_filetest.gno +++ b/examples/gno.land/r/demo/wugnot/z0_filetest.gno @@ -41,16 +41,10 @@ func printBalances() { printSingleBalance := func(name string, addr std.Address) { wugnotBal := wugnot.BalanceOf(pusers.AddressOrName(addr)) std.TestSetOrigCaller(addr) - abanker := std.GetBanker(std.BankerTypeOrigSend) - acoins := abanker.GetCoins(addr).AmountOf("ugnot") - bbanker := std.GetBanker(std.BankerTypeRealmIssue) - bcoins := bbanker.GetCoins(addr).AmountOf("ugnot") - cbanker := std.GetBanker(std.BankerTypeRealmSend) - ccoins := cbanker.GetCoins(addr).AmountOf("ugnot") - dbanker := std.GetBanker(std.BankerTypeReadonly) - dcoins := dbanker.GetCoins(addr).AmountOf("ugnot") - fmt.Printf("| %-13s | addr=%s | wugnot=%-5d | ugnot=%-9d %-9d %-9d %-9d |\n", - name, addr, wugnotBal, acoins, bcoins, ccoins, dcoins) + robanker := std.GetBanker(std.BankerTypeReadonly) + coins := robanker.GetCoins(addr).AmountOf("ugnot") + fmt.Printf("| %-13s | addr=%s | wugnot=%-5d | ugnot=%-9d |\n", + name, addr, wugnotBal, coins) } println("-----------") printSingleBalance("wugnot_test", addrt) @@ -61,17 +55,17 @@ func printBalances() { // Output: // ----------- -// | wugnot_test | addr=g19rmydykafrqyyegc8uuaxxpzqwzcnxraj2dev9 | wugnot=0 | ugnot=200000000 200000000 200000000 200000000 | -// | wugnot | addr=g1pf6dv9fjk3rn0m4jjcne306ga4he3mzmupfjl6 | wugnot=0 | ugnot=100000001 100000001 100000001 100000001 | -// | addr1 | addr=g1w3jhxap3ta047h6lta047h6lta047h6l4mfnm7 | wugnot=0 | ugnot=100000001 100000001 100000001 100000001 | +// | wugnot_test | addr=g19rmydykafrqyyegc8uuaxxpzqwzcnxraj2dev9 | wugnot=0 | ugnot=200000000 | +// | wugnot | addr=g1pf6dv9fjk3rn0m4jjcne306ga4he3mzmupfjl6 | wugnot=0 | ugnot=100000001 | +// | addr1 | addr=g1w3jhxap3ta047h6lta047h6lta047h6l4mfnm7 | wugnot=0 | ugnot=100000001 | // ----------- // ----------- -// | wugnot_test | addr=g19rmydykafrqyyegc8uuaxxpzqwzcnxraj2dev9 | wugnot=123400 | ugnot=200000000 200000000 200000000 200000000 | -// | wugnot | addr=g1pf6dv9fjk3rn0m4jjcne306ga4he3mzmupfjl6 | wugnot=0 | ugnot=100000001 100000001 100000001 100000001 | -// | addr1 | addr=g1w3jhxap3ta047h6lta047h6lta047h6l4mfnm7 | wugnot=0 | ugnot=100000001 100000001 100000001 100000001 | +// | wugnot_test | addr=g19rmydykafrqyyegc8uuaxxpzqwzcnxraj2dev9 | wugnot=123400 | ugnot=200000000 | +// | wugnot | addr=g1pf6dv9fjk3rn0m4jjcne306ga4he3mzmupfjl6 | wugnot=0 | ugnot=100000001 | +// | addr1 | addr=g1w3jhxap3ta047h6lta047h6lta047h6l4mfnm7 | wugnot=0 | ugnot=100000001 | // ----------- // ----------- -// | wugnot_test | addr=g19rmydykafrqyyegc8uuaxxpzqwzcnxraj2dev9 | wugnot=119158 | ugnot=200004242 200004242 200004242 200004242 | -// | wugnot | addr=g1pf6dv9fjk3rn0m4jjcne306ga4he3mzmupfjl6 | wugnot=0 | ugnot=99995759 99995759 99995759 99995759 | -// | addr1 | addr=g1w3jhxap3ta047h6lta047h6lta047h6l4mfnm7 | wugnot=0 | ugnot=100000001 100000001 100000001 100000001 | +// | wugnot_test | addr=g19rmydykafrqyyegc8uuaxxpzqwzcnxraj2dev9 | wugnot=119158 | ugnot=200004242 | +// | wugnot | addr=g1pf6dv9fjk3rn0m4jjcne306ga4he3mzmupfjl6 | wugnot=0 | ugnot=99995759 | +// | addr1 | addr=g1w3jhxap3ta047h6lta047h6lta047h6l4mfnm7 | wugnot=0 | ugnot=100000001 | // ----------- diff --git a/gnovm/stdlibs/std/banker.gno b/gnovm/stdlibs/std/banker.gno index 958fea369a3..2b94292bd7e 100644 --- a/gnovm/stdlibs/std/banker.gno +++ b/gnovm/stdlibs/std/banker.gno @@ -1,6 +1,8 @@ package std -import "strconv" +import ( + "strconv" +) // Realm functions can call std.GetBanker(options) to get // a banker instance. Banker objects cannot be persisted, @@ -66,7 +68,20 @@ func GetBanker(bt BankerType) Banker { if bt >= maxBanker { panic("invalid banker type") } - return banker{bt} + + var pkgAddr Address + if bt == BankerTypeOrigSend { + pkgAddr = GetOrigPkgAddr() + if pkgAddr != CurrentRealm().Addr() { + panic("banker with type BankerTypeOrigSend can only be instantiated by the origin package") + } + } else if bt == BankerTypeRealmSend || bt == BankerTypeRealmIssue { + pkgAddr = CurrentRealm().Addr() + } + return banker{ + bt, + pkgAddr, + } } // These are native bindings to the banker's functions. @@ -77,7 +92,8 @@ func bankerIssueCoin(bt uint8, addr string, denom string, amount int64) func bankerRemoveCoin(bt uint8, addr string, denom string, amount int64) type banker struct { - bt BankerType + bt BankerType + pkgAddr Address } func (b banker) GetCoins(addr Address) (dst Coins) { @@ -93,6 +109,10 @@ func (b banker) SendCoins(from, to Address, amt Coins) { if b.bt == BankerTypeReadonly { panic("BankerTypeReadonly cannot send coins") } + if b.pkgAddr != from { + msg := `can only send coins from realm that created banker "` + b.pkgAddr + `", not "` + from + `"` + panic(msg) + } denoms, amounts := amt.expandNative() bankerSendCoins(uint8(b.bt), string(from), string(to), denoms, amounts) } diff --git a/gnovm/stdlibs/std/banker.go b/gnovm/stdlibs/std/banker.go index c7747d2a5a6..892af94777f 100644 --- a/gnovm/stdlibs/std/banker.go +++ b/gnovm/stdlibs/std/banker.go @@ -48,23 +48,6 @@ func X_bankerSendCoins(m *gno.Machine, bt uint8, fromS, toS string, denoms []str amt := CompactCoins(denoms, amounts) from, to := crypto.Bech32Address(fromS), crypto.Bech32Address(toS) - pkgAddr := ctx.OrigPkgAddr - if m.Realm != nil { - pkgPath := m.Realm.Path - pkgAddr = gno.DerivePkgAddr(pkgPath).Bech32() - } - - if bt == btOrigSend || bt == btRealmSend { - if from != pkgAddr { - m.Panic(typedString( - fmt.Sprintf( - "can only send from the realm package address %q, but got %q", - pkgAddr, from), - )) - return - } - } - switch bt { case btOrigSend: // indirection allows us to "commit" in a second phase