[GnoVM] Allow banker to be initialized in MsgRun

[leohhhn]

Description

When users send funds along with MsgRun, they should use the banker to then forward those funds to specific addresses or contracts in the script itself. This is currently not possible as the ephemeral run realm is not considered a "realm" and the VM panics with VM panic: caller is not a realm.

Also, the funds that remain in the balance of the ephemeral realm should be refunded to the user that created it after the transaction.

cc @moul

[clockworkgr]

Agreed, from what I can tell the issue is with the realm assertion.

Also, the funds that remain in the balance of the ephemeral realm should be refunded to the user that created it after the transaction.

I think this is a non-issue as if I'm not mistaken, the ephmeral realm's address is the user's own address. --send does not actualy move coins in the Run scenario but simply functions as a limit of what the Run script can spend. Is that right @moul ?

[tbruyelle]

Agreed, from what I can tell the issue is with the realm assertion.

Also, the funds that remain in the balance of the ephemeral realm should be refunded to the user that created it after the transaction.

I think this is a non-issue as if I'm not mistaken, the ephmeral realm's address is the user's own address. --send does not actualy move coins in the Run scenario but simply functions as a limit of what the Run script can spend. Is that right @moul ?

Yes, with MsgRun, the caller address and the script address are the same, so the -send has no effect, the funds are not moving. This discards the should be refunded requirement.

So we don't need the -send flag in maketx run, and probably #4866 should be closed.

Inside the script, we can just transfer the funds to whatever realm using:

	bank := banker.NewBanker(banker.BankerTypeRealmIssue)
	ibcCoreAddr := chain.PackageAddress("gno.land/r/aib/ibc/core")
	bank.SendCoins(
		runtime.CurrentRealm().Address(), ibcCoreAddr, chain.NewCoins(chain.NewCoin("ugnot", 1000)),
	)

Note that I used a RealmIssue banker type since I don't want the funds to be limited to what have been put in the -send flag.

Note this still requires NewBanker to be updated with the following:

diff --git a/gnovm/stdlibs/chain/banker/banker.gno b/gnovm/stdlibs/chain/banker/banker.gno
index 6f6effad8..b7850466d 100644
--- a/gnovm/stdlibs/chain/banker/banker.gno
+++ b/gnovm/stdlibs/chain/banker/banker.gno
@@ -68,7 +68,9 @@ func (b BankerType) String() string {
 // NewBanker returns a new Banker, with its capabilities matching the given
 // [BankerType].
 func NewBanker(bt BankerType) Banker {
-	assertCallerIsRealm()
+	if !runtime.CurrentRealm().IsCode() {
+		panic("caller is not code")
+	}
 	if bt >= maxBanker {
 		panic("invalid banker type")
 	}

Note though that I'm not sure that this change is secure. Realm.IsCode returns true for realm and for ephemeral (== MsgRun script). What about packages ?

// IsCode returns true if r.pkgPath is not empty.
// - true if Coderealm and !IsEphemeral, an uploaded code realm.
// - true if CodeRealm and IsEphemeral, 'gnokey maketx run'.
// - false if UserRealm, 'gnokey maketx call'.
func (r Realm) IsCode() bool {
	return r.pkgPath != ""
}

[leohhhn]

Packages cannot initialize the Banker module

[clockworkgr]

Agreed, from what I can tell the issue is with the realm assertion.

Also, the funds that remain in the balance of the ephemeral realm should be refunded to the user that created it after the transaction.

I think this is a non-issue as if I'm not mistaken, the ephmeral realm's address is the user's own address. --send does not actualy move coins in the Run scenario but simply functions as a limit of what the Run script can spend. Is that right @moul ?

Yes, with MsgRun, the caller address and the script address are the same, so the -send has no effect, the funds are not moving. This discards the should be refunded requirement.

So we don't need the -send flag in maketx run, and probably #4866 should be closed.

Disagree, I havent followed the code thoroughly but I believe the --send functions (and is intended to function) as a limit of what the code can spend. Thus its more secure to match --send with the amount in the IBC transfer.

Inside the script, we can just transfer the funds to whatever realm using:

bank := banker.NewBanker(banker.BankerTypeRealmIssue)
ibcCoreAddr := chain.PackageAddress("gno.land/r/aib/ibc/core")
bank.SendCoins(
runtime.CurrentRealm().Address(), ibcCoreAddr, chain.NewCoins(chain.NewCoin("ugnot", 1000)),
)
Note that I used a RealmIssue banker type since I don't want the funds to be limited to what have been put in the -send flag.

See above

Note this still requires NewBanker to be updated with the following:

diff --git a/gnovm/stdlibs/chain/banker/banker.gno b/gnovm/stdlibs/chain/banker/banker.gno
index 6f6effad8..b7850466d 100644
--- a/gnovm/stdlibs/chain/banker/banker.gno
+++ b/gnovm/stdlibs/chain/banker/banker.gno
@@ -68,7 +68,9 @@ func (b BankerType) String() string {
// NewBanker returns a new Banker, with its capabilities matching the given
// [BankerType].
func NewBanker(bt BankerType) Banker {

• assertCallerIsRealm()

• if !runtime.CurrentRealm().IsCode() {

• panic("caller is not code")
  

• }
  if bt >= maxBanker {
  panic("invalid banker type")
  }
  Note though that I'm not sure that this change is secure. Realm.IsCode returns true for realm and for ephemeral (== MsgRun script). What about packages ?

// IsCode returns true if r.pkgPath is not empty.
// - true if Coderealm and !IsEphemeral, an uploaded code realm.
// - true if CodeRealm and IsEphemeral, 'gnokey maketx run'.
// - false if UserRealm, 'gnokey maketx call'.
func (r Realm) IsCode() bool {
return r.pkgPath != ""
}

[tbruyelle]

Packages cannot initialize the Banker module

But with the suggested change it can, I need to find something else than IsCode().

Disagree, I havent followed the code thoroughly but I believe the --send functions (and is intended to function) as a limit of what the code can spend. Thus its more secure to match --send with the amount in the IBC transfer.

Right, I was following the idea of run scripts being owned by the user, and so user being responsible for their content. But more likely run scripts will be generated by some dapps and users are not going to check their content. Hence -send makes sense as a visible wallet parameter.

[clockworkgr]

Packages cannot initialize the Banker module

But with the suggested change it can, I need to find something else than IsCode().

Disagree, I havent followed the code thoroughly but I believe the --send functions (and is intended to function) as a limit of what the code can spend. Thus its more secure to match --send with the amount in the IBC transfer.

Right, I was following the idea of run scripts being owned by the user, and so user being responsible for their content. But more likely run scripts will be generated by some dapps and users are not going to check their content. Hence -send makes sense as a visible wallet parameter.

Copying from TG:

From what I've grokked so far, it seems to me that if we DO want run scripts to be able to instantiate and use a banker this conditional here that causes the error we've encountered:

gno/gnovm/stdlibs/chain/banker/banker.go

Line 101 in 1218753

if path := frame.LastPackage.PkgPath; !gno.IsRealmPath(path) {

should be:
!gno.IsRealmPath(path) && !gno.IsEphemeralPath(path)

wdyt ?

[tbruyelle]

Yes, this is the way, but with your comment you also make me realize something else. As a security measure we should also enforce that BankerTypeRealmIssue cannot be used in run scripts. This way only the -send amount can be transfered in the script and nothing more.

[tbruyelle]

Thanks for approving, I'm working on it