From 9458b72866a796d557df7a35c427df4d9b9f3d85 Mon Sep 17 00:00:00 2001 From: Thomas Bruyelle Date: Sun, 18 Feb 2024 18:51:54 +0100 Subject: [PATCH] feat: make `AssertOriginCall` always panic with `MsgRun` Fix #1663 `AssertOriginCall` used to panic on `MsgRun` because it involves more than 2 frames. But we want to reinforce that property, with an additional check. Added an improved version of the unit and txtar tests from #1048. In particular, the txtar adds 2 more cases : 19) MsgCall invokes std.AssertOriginCall directly: pass 20) MsgRun invokes std.AssertOriginCall directly: PANIC Note that 19) involves a change in the AssertOriginCall algorithm, because in that situation there's only a single frame. Even if there's no reason to call `std.AssertOriginCall()` directly from the command line, I think it's logic to make it pass, because that's a origin call. --- .../r/demo/tests/subtests/subtests.gno | 8 + examples/gno.land/r/demo/tests/tests.gno | 12 +- examples/gno.land/r/demo/tests/tests_test.gno | 36 ++- .../gnoland/testdata/assertorigincall.txtar | 254 ++++++++++++++++++ gnovm/stdlibs/std/native.gno | 12 +- gnovm/stdlibs/std/native.go | 8 +- gnovm/stdlibs/std/native_test.go | 215 +++++++++++++++ 7 files changed, 528 insertions(+), 17 deletions(-) create mode 100644 gno.land/cmd/gnoland/testdata/assertorigincall.txtar create mode 100644 gnovm/stdlibs/std/native_test.go diff --git a/examples/gno.land/r/demo/tests/subtests/subtests.gno b/examples/gno.land/r/demo/tests/subtests/subtests.gno index e8796a73081..5cc4d1bea16 100644 --- a/examples/gno.land/r/demo/tests/subtests/subtests.gno +++ b/examples/gno.land/r/demo/tests/subtests/subtests.gno @@ -15,3 +15,11 @@ func GetPrevRealm() std.Realm { func Exec(fn func()) { fn() } + +func AssertOriginCall() { + std.AssertOriginCall() +} + +func IsOriginCall() bool { + return std.IsOriginCall() +} diff --git a/examples/gno.land/r/demo/tests/tests.gno b/examples/gno.land/r/demo/tests/tests.gno index 0094ad2ae35..34a564e17b3 100644 --- a/examples/gno.land/r/demo/tests/tests.gno +++ b/examples/gno.land/r/demo/tests/tests.gno @@ -20,14 +20,22 @@ func CurrentRealmPath() string { return std.CurrentRealmPath() } -func AssertOriginCall() { +func CallAssertOriginCall() { std.AssertOriginCall() } -func IsOriginCall() bool { +func CallIsOriginCall() bool { return std.IsOriginCall() } +func CallSubtestsAssertOriginCall() { + rsubtests.AssertOriginCall() +} + +func CallSubtestsIsOriginCall() bool { + return rsubtests.IsOriginCall() +} + //---------------------------------------- // Test structure to ensure cross-realm modification is prevented. diff --git a/examples/gno.land/r/demo/tests/tests_test.gno b/examples/gno.land/r/demo/tests/tests_test.gno index 3dcbeecf18c..41d89526a4b 100644 --- a/examples/gno.land/r/demo/tests/tests_test.gno +++ b/examples/gno.land/r/demo/tests/tests_test.gno @@ -8,28 +8,40 @@ import ( ) func TestAssertOriginCall(t *testing.T) { - // No-panic case - AssertOriginCall() - if !IsOriginCall() { + // CallAssertOriginCall(): no panic + CallAssertOriginCall() + if !CallIsOriginCall() { t.Errorf("expected IsOriginCall=true but got false") } - // Panic case + // CallAssertOriginCall() from a block: panic expectedReason := "invalid non-origin call" + func() { + defer func() { + r := recover() + if r == nil || r.(string) != expectedReason { + t.Errorf("expected panic with '%v', got '%v'", expectedReason, r) + } + }() + // if called inside a function literal, this is no longer an origin call + // because there's one additional frame (the function literal block). + if CallIsOriginCall() { + t.Errorf("expected IsOriginCall=false but got true") + } + CallAssertOriginCall() + }() + + // CallSubtestsAssertOriginCall(): panic defer func() { r := recover() if r == nil || r.(string) != expectedReason { t.Errorf("expected panic with '%v', got '%v'", expectedReason, r) } }() - func() { - // if called inside a function literal, this is no longer an origin call - // because there's one additional frame (the function literal). - if IsOriginCall() { - t.Errorf("expected IsOriginCall=false but got true") - } - AssertOriginCall() - }() + if CallSubtestsIsOriginCall() { + t.Errorf("expected IsOriginCall=false but got true") + } + CallSubtestsAssertOriginCall() } func TestPrevRealm(t *testing.T) { diff --git a/gno.land/cmd/gnoland/testdata/assertorigincall.txtar b/gno.land/cmd/gnoland/testdata/assertorigincall.txtar new file mode 100644 index 00000000000..832724eea20 --- /dev/null +++ b/gno.land/cmd/gnoland/testdata/assertorigincall.txtar @@ -0,0 +1,254 @@ +# This test ensures the consistency of the std.AssertOriginCall function, in +# the following situations: +# +# | Num | Msg Type | Call from | Entry Point | Result | +# |-----|:--------:|:-------------------:|:----------------------:|:------:| +# | 1 | MsgCall | wallet direct | myrealm.A() | PANIC | +# | 2 | | | myrealm.B() | pass | +# | 3 | | | myrealm.C() | pass | +# | 4 | | through /r/foo | myrealm.A() | PANIC | +# | 5 | | | myrealm.B() | pass | +# | 6 | | | myrealm.C() | PANIC | +# | 7 | | through /p/demo/bar | myrealm.A() | PANIC | +# | 8 | | | myrealm.B() | pass | +# | 9 | | | myrealm.C() | PANIC | +# | 10 | MsgRun | wallet direct | myrealm.A() | PANIC | +# | 11 | | | myrealm.B() | pass | +# | 12 | | | myrealm.C() | PANIC | +# | 13 | | through /r/foo | myrealm.A() | PANIC | +# | 14 | | | myrealm.B() | pass | +# | 15 | | | myrealm.C() | PANIC | +# | 16 | | through /p/demo/bar | myrealm.A() | PANIC | +# | 17 | | | myrealm.B() | pass | +# | 18 | | | myrealm.C() | PANIC | +# | 19 | MsgCall | wallet direct | std.AssertOriginCall() | pass | +# | 20 | MsgRun | wallet direct | std.AssertOriginCall() | PANIC | + +# Init +## start a new node +gnoland start + +## deploy myrlm +gnokey maketx addpkg -pkgdir $WORK/r/myrlm -pkgpath gno.land/r/myrlm -gas-fee 1000000ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test test1 +stdout 'OK!' + +## deploy r/foo +gnokey maketx addpkg -pkgdir $WORK/r/foo -pkgpath gno.land/r/foo -gas-fee 1000000ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test test1 +stdout 'OK!' + +## deploy p/demo/bar +gnokey maketx addpkg -pkgdir $WORK/p/demo/bar -pkgpath gno.land/p/demo/bar -gas-fee 1000000ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test test1 +stdout 'OK!' + +# Test cases +## 1. MsgCall -> myrlm.A: PANIC +! gnokey maketx call -pkgpath gno.land/r/myrlm -func A -gas-fee 100000ugnot -gas-wanted 2000000 -broadcast -chainid tendermint_test test1 +stderr 'invalid non-origin call' + +## 2. MsgCall -> myrlm.B: PASS +gnokey maketx call -pkgpath gno.land/r/myrlm -func B -gas-fee 100000ugnot -gas-wanted 2000000 -broadcast -chainid tendermint_test test1 +stdout 'OK!' + +## 3. MsgCall -> myrlm.C: PASS +gnokey maketx call -pkgpath gno.land/r/myrlm -func C -gas-fee 100000ugnot -gas-wanted 2000000 -broadcast -chainid tendermint_test test1 +stdout 'OK!' + +## 4. MsgCall -> r/foo.A -> myrlm.A: PANIC +! gnokey maketx call -pkgpath gno.land/r/foo -func A -gas-fee 100000ugnot -gas-wanted 2000000 -broadcast -chainid tendermint_test test1 +stderr 'invalid non-origin call' + +## 5. MsgCall -> r/foo.B -> myrlm.B: PASS +gnokey maketx call -pkgpath gno.land/r/foo -func B -gas-fee 100000ugnot -gas-wanted 2000000 -broadcast -chainid tendermint_test test1 +stdout 'OK!' + +## 6. MsgCall -> r/foo.C -> myrlm.C: PANIC +! gnokey maketx call -pkgpath gno.land/r/foo -func C -gas-fee 100000ugnot -gas-wanted 2000000 -broadcast -chainid tendermint_test test1 +stderr 'invalid non-origin call' + +## 7. MsgCall -> p/demo/bar.A -> myrlm.A: PANIC +! gnokey maketx call -pkgpath gno.land/p/demo/bar -func A -gas-fee 100000ugnot -gas-wanted 2000000 -broadcast -chainid tendermint_test test1 +stderr 'invalid non-origin call' + +## 8. MsgCall -> p/demo/bar.B -> myrlm.B: PASS +gnokey maketx call -pkgpath gno.land/p/demo/bar -func B -gas-fee 100000ugnot -gas-wanted 2000000 -broadcast -chainid tendermint_test test1 +stdout 'OK!' + +## 9. MsgCall -> p/demo/bar.C -> myrlm.C: PANIC +! gnokey maketx call -pkgpath gno.land/p/demo/bar -func C -gas-fee 100000ugnot -gas-wanted 2000000 -broadcast -chainid tendermint_test test1 +stderr 'invalid non-origin call' + +## 10. MsgRun -> run.main -> myrlm.A: PANIC +! gnokey maketx run -gas-fee 100000ugnot -gas-wanted 2000000 -broadcast -chainid tendermint_test test1 $WORK/run/myrlmA.gno +stderr 'invalid non-origin call' + +## 11. MsgRun -> run.main -> myrlm.B: PASS +gnokey maketx run -gas-fee 100000ugnot -gas-wanted 2000000 -broadcast -chainid tendermint_test test1 $WORK/run/myrlmB.gno +stdout 'OK!' + +## 12. MsgRun -> run.main -> myrlm.C: PANIC +! gnokey maketx run -gas-fee 100000ugnot -gas-wanted 2000000 -broadcast -chainid tendermint_test test1 $WORK/run/myrlmC.gno +stderr 'invalid non-origin call' + +## 13. MsgRun -> run.main -> foo.A: PANIC +! gnokey maketx run -gas-fee 100000ugnot -gas-wanted 2000000 -broadcast -chainid tendermint_test test1 $WORK/run/fooA.gno +stderr 'invalid non-origin call' + +## 14. MsgRun -> run.main -> foo.B: PASS +gnokey maketx run -gas-fee 100000ugnot -gas-wanted 2000000 -broadcast -chainid tendermint_test test1 $WORK/run/fooB.gno +stdout 'OK!' + +## 15. MsgRun -> run.main -> foo.C: PANIC +! gnokey maketx run -gas-fee 100000ugnot -gas-wanted 2000000 -broadcast -chainid tendermint_test test1 $WORK/run/fooC.gno +stderr 'invalid non-origin call' + +## 16. MsgRun -> run.main -> bar.A: PANIC +! gnokey maketx run -gas-fee 100000ugnot -gas-wanted 2000000 -broadcast -chainid tendermint_test test1 $WORK/run/barA.gno +stderr 'invalid non-origin call' + +## 17. MsgRun -> run.main -> bar.B: PASS +gnokey maketx run -gas-fee 100000ugnot -gas-wanted 2000000 -broadcast -chainid tendermint_test test1 $WORK/run/barB.gno +stdout 'OK!' + +## 18. MsgRun -> run.main -> bar.C: PANIC +! gnokey maketx run -gas-fee 100000ugnot -gas-wanted 2000000 -broadcast -chainid tendermint_test test1 $WORK/run/barC.gno +stderr 'invalid non-origin call' + +## 19. MsgCall -> std.AssertOriginCall: pass +gnokey maketx call -pkgpath std -func AssertOriginCall -gas-fee 100000ugnot -gas-wanted 2000000 -broadcast -chainid tendermint_test test1 +stdout 'OK!' + +## 20. MsgRun -> std.AssertOriginCall: PANIC +! gnokey maketx run -gas-fee 100000ugnot -gas-wanted 2000000 -broadcast -chainid tendermint_test test1 $WORK/run/baz.gno +stderr 'invalid non-origin call' + + +-- r/myrlm/rlm.gno -- +package myrlm + +import "std" + +func A() { + C() +} + +func B() { + if false { + C() + } +} + +func C() { + std.AssertOriginCall() +} +-- r/foo/foo.gno -- +package foo + +import "gno.land/r/myrlm" + +func A() { + myrlm.A() +} + +func B() { + myrlm.B() +} + +func C() { + myrlm.C() +} +-- p/demo/bar/bar.gno -- +package bar + +import "gno.land/r/myrlm" + +func A() { + myrlm.A() +} + +func B() { + myrlm.B() +} + +func C() { + myrlm.C() +} +-- run/myrlmA.gno -- +package main + +import myrlm "gno.land/r/myrlm" + +func main() { + myrlm.A() +} +-- run/myrlmB.gno -- +package main + +import "gno.land/r/myrlm" + +func main() { + myrlm.B() +} +-- run/myrlmC.gno -- +package main + +import "gno.land/r/myrlm" + +func main() { + myrlm.C() +} +-- run/fooA.gno -- +package main + +import "gno.land/r/foo" + +func main() { + foo.A() +} +-- run/fooB.gno -- +package main + +import "gno.land/r/foo" + +func main() { + foo.B() +} +-- run/fooC.gno -- +package main + +import "gno.land/r/foo" + +func main() { + foo.C() +} +-- run/barA.gno -- +package main + +import "gno.land/p/demo/bar" + +func main() { + bar.A() +} +-- run/barB.gno -- +package main + +import "gno.land/p/demo/bar" + +func main() { + bar.B() +} +-- run/barC.gno -- +package main + +import "gno.land/p/demo/bar" + +func main() { + bar.C() +} +-- run/baz.gno -- +package main + +import "std" + +func main() { + std.AssertOriginCall() +} diff --git a/gnovm/stdlibs/std/native.gno b/gnovm/stdlibs/std/native.gno index 2f7da810bcb..66a9e99e2c5 100644 --- a/gnovm/stdlibs/std/native.gno +++ b/gnovm/stdlibs/std/native.gno @@ -1,7 +1,15 @@ package std -func AssertOriginCall() // injected -func IsOriginCall() bool // injected +// AssertOriginCall panics is [IsOriginCall] return false. +func AssertOriginCall() // injected + +// IsOriginCall return true only if the calling method is invoked via a direct +// MsgCall. It returns false for all other cases, like if the calling method +// is invoked by an other method (even from the same realm or package). +// It also returns false every time when the transaction is broadcasted via +// MsgRun. +func IsOriginCall() bool // injected + func CurrentRealmPath() string // injected func GetChainID() string // injected func GetHeight() int64 // injected diff --git a/gnovm/stdlibs/std/native.go b/gnovm/stdlibs/std/native.go index 044badaa308..e6f0f42c9e1 100644 --- a/gnovm/stdlibs/std/native.go +++ b/gnovm/stdlibs/std/native.go @@ -16,7 +16,13 @@ func AssertOriginCall(m *gno.Machine) { } func IsOriginCall(m *gno.Machine) bool { - return len(m.Frames) == 2 + n := m.NumFrames() + if n == 0 { + return false + } + firstPkg := m.Frames[0].LastPackage + isMsgCall := firstPkg != nil && firstPkg.PkgPath == "main" + return n <= 2 && isMsgCall } func CurrentRealmPath(m *gno.Machine) string { diff --git a/gnovm/stdlibs/std/native_test.go b/gnovm/stdlibs/std/native_test.go new file mode 100644 index 00000000000..4a34559fbfb --- /dev/null +++ b/gnovm/stdlibs/std/native_test.go @@ -0,0 +1,215 @@ +package std + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + gno "github.com/gnolang/gno/gnovm/pkg/gnolang" +) + +func TestPrevRealmIsOrigin(t *testing.T) { + var ( + user = gno.DerivePkgAddr("user1.gno").Bech32() + ctx = ExecContext{ + OrigCaller: user, + } + msgCallFrame = gno.Frame{LastPackage: &gno.PackageValue{PkgPath: "main"}} + msgRunFrame = gno.Frame{LastPackage: &gno.PackageValue{PkgPath: "gno.land/r/user-realm"}} + ) + tests := []struct { + name string + machine *gno.Machine + expectedRealm Realm + expectedIsOriginCall bool + }{ + { + name: "no frames", + machine: &gno.Machine{ + Context: ctx, + Frames: []gno.Frame{}, + }, + expectedRealm: Realm{ + addr: user, + pkgPath: "", + }, + expectedIsOriginCall: false, + }, + { + name: "one frame w/o LastPackage", + machine: &gno.Machine{ + Context: ctx, + Frames: []gno.Frame{ + {LastPackage: nil}, + }, + }, + expectedRealm: Realm{ + addr: user, + pkgPath: "", + }, + expectedIsOriginCall: false, + }, + { + name: "one package frame", + machine: &gno.Machine{ + Context: ctx, + Frames: []gno.Frame{ + {LastPackage: &gno.PackageValue{PkgPath: "gno.land/p/xxx"}}, + }, + }, + expectedRealm: Realm{ + addr: user, + pkgPath: "", + }, + expectedIsOriginCall: false, + }, + { + name: "one realm frame", + machine: &gno.Machine{ + Context: ctx, + Frames: []gno.Frame{ + {LastPackage: &gno.PackageValue{PkgPath: "gno.land/r/xxx"}}, + }, + }, + expectedRealm: Realm{ + addr: user, + pkgPath: "", + }, + expectedIsOriginCall: false, + }, + { + name: "one msgCall frame", + machine: &gno.Machine{ + Context: ctx, + Frames: []gno.Frame{ + msgCallFrame, + }, + }, + expectedRealm: Realm{ + addr: user, + pkgPath: "", + }, + expectedIsOriginCall: true, + }, + { + name: "one msgRun frame", + machine: &gno.Machine{ + Context: ctx, + Frames: []gno.Frame{ + msgRunFrame, + }, + }, + expectedRealm: Realm{ + addr: user, + pkgPath: "", + }, + expectedIsOriginCall: false, + }, + { + name: "one package frame and one msgCall frame", + machine: &gno.Machine{ + Context: ctx, + Frames: []gno.Frame{ + msgCallFrame, + {LastPackage: &gno.PackageValue{PkgPath: "gno.land/p/xxx"}}, + }, + }, + expectedRealm: Realm{ + addr: user, + pkgPath: "", + }, + expectedIsOriginCall: true, + }, + { + name: "one realm frame and one msgCall frame", + machine: &gno.Machine{ + Context: ctx, + Frames: []gno.Frame{ + msgCallFrame, + {LastPackage: &gno.PackageValue{PkgPath: "gno.land/r/xxx"}}, + }, + }, + expectedRealm: Realm{ + addr: user, + pkgPath: "", + }, + expectedIsOriginCall: true, + }, + { + name: "one package frame and one msgRun frame", + machine: &gno.Machine{ + Context: ctx, + Frames: []gno.Frame{ + msgRunFrame, + {LastPackage: &gno.PackageValue{PkgPath: "gno.land/p/xxx"}}, + }, + }, + expectedRealm: Realm{ + addr: user, + pkgPath: "", + }, + expectedIsOriginCall: false, + }, + { + name: "one realm frame and one msgRun frame", + machine: &gno.Machine{ + Context: ctx, + Frames: []gno.Frame{ + msgRunFrame, + {LastPackage: &gno.PackageValue{PkgPath: "gno.land/r/xxx"}}, + }, + }, + expectedRealm: Realm{ + addr: user, + pkgPath: "", + }, + expectedIsOriginCall: false, + }, + { + name: "multiple frames with one realm", + machine: &gno.Machine{ + Context: ctx, + Frames: []gno.Frame{ + {LastPackage: &gno.PackageValue{PkgPath: "gno.land/p/xxx"}}, + {LastPackage: &gno.PackageValue{PkgPath: "gno.land/p/xxx"}}, + {LastPackage: &gno.PackageValue{PkgPath: "gno.land/r/xxx"}}, + }, + }, + expectedRealm: Realm{ + addr: user, + pkgPath: "", + }, + expectedIsOriginCall: false, + }, + { + name: "multiple frames with multiple realms", + machine: &gno.Machine{ + Context: ctx, + Frames: []gno.Frame{ + {LastPackage: &gno.PackageValue{PkgPath: "gno.land/r/zzz"}}, + {LastPackage: &gno.PackageValue{PkgPath: "gno.land/r/zzz"}}, + {LastPackage: &gno.PackageValue{PkgPath: "gno.land/r/yyy"}}, + {LastPackage: &gno.PackageValue{PkgPath: "gno.land/p/yyy"}}, + {LastPackage: &gno.PackageValue{PkgPath: "gno.land/p/xxx"}}, + {LastPackage: &gno.PackageValue{PkgPath: "gno.land/r/xxx"}}, + }, + }, + expectedRealm: Realm{ + addr: gno.DerivePkgAddr("gno.land/r/yyy").Bech32(), + pkgPath: "gno.land/r/yyy", + }, + expectedIsOriginCall: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert := assert.New(t) + + realm := PrevRealm(tt.machine) + isOrigin := IsOriginCall(tt.machine) + + assert.Equal(tt.expectedRealm, realm) + assert.Equal(tt.expectedIsOriginCall, isOrigin) + }) + } +}