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.

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

[GnoVM] OrigSend for realms called from another realm #1887

Closed
r3v4s opened this issue Apr 4, 2024 · 5 comments
Closed

[GnoVM] OrigSend for realms called from another realm #1887

r3v4s opened this issue Apr 4, 2024 · 5 comments
Assignees
Labels
help wanted Want to contribute? We recommend these issues. 📦 🤖 gnovm Issues or PRs gnovm related

Comments

@r3v4s
Copy link
Contributor

r3v4s commented Apr 4, 2024

FYI, std.GetOrigSend() will return very first coin that being sent in current stack frame.

For example,

  1. User -> ContractA // send 1234ugnot
  2. (Automatically) ContractA -> ContractB // send 999ugnot

In above case, '1234ugnot' is origSend but what about '999ugnot'?
As view of ContractB, is there ways to parse 999ugnot (which isn't orig send) ??

i.e looking for something like (GetOrigCaller vs PrevRelm.Addr) (GetOrigSend vs XXXX)

txtar testing

loadpkg gno.land/p/demo/ufmt

gnoland start

gnokey maketx addpkg -pkgdir $WORK/second -pkgpath gno.land/r/second -gas-fee 1000000ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test test1

gnokey maketx addpkg -pkgdir $WORK/first -pkgpath gno.land/r/first -gas-fee 1000000ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test test1

gnokey maketx call -pkgpath gno.land/r/first -func Receive -send 1234ugnot -gas-fee 1000000ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test test1


-- first/realm.gno --
package first

import (
    "std"
    "gno.land/p/demo/ufmt"

    "gno.land/r/second"
)

func Receive() (string, int64) {
    // receive ugnot from user and send to another realm
    return SendTo()
}

func SendTo() (string, int64) {
    banker := std.GetBanker(std.BankerTypeRealmSend)
    send := std.Coins{{"ugnot", int64(999)}}
    banker.SendCoins(std.CurrentRealm().Addr(), std.DerivePkgAddr("gno.land/r/second"), send)

    return second.Receive()
}

-- second/realm.gno --
package second

import (
    "std"
)

func Receive() (string, int64) {
    coins := std.GetOrigSend() // origSend will be '1234ugnot' 
    // TODO: ways to parse 999ugnot ??
    coin := coins[0]

    return coin.Denom, coin.Amount
}
@r3v4s r3v4s self-assigned this Apr 4, 2024
@leohhhn
Copy link
Contributor

leohhhn commented Apr 4, 2024

Hey, thank you for bringing this up - as far as I know, we do not have a way to do this right now, which is very bizarre considering it is a much needed feature most apps, let alone a DeFi app.

For anyone reading - to draw an analogy:
Getting the origin caller, and the previous caller is not the same - this is why we have GetOrigCaller & PrevRealm.
The same needs to exist, but for coins.

I will add this to the next engineering call to be discussed. Thank you @r3v4s again for finding this issue.

EDIT: thinking about this a little more, there should only be one type of parsing coins - parsing coins from the immediate, previous caller (similar to what msg.value is in EVM). In @r3v4s's case, this would mean that the first realm only has access to coins sent in by the user, and the second realm only has access to coins sent in by the first realm.

@leohhhn leohhhn added the 📦 🤖 gnovm Issues or PRs gnovm related label Jun 17, 2024
@leohhhn leohhhn changed the title Ways to parse coins that sent directly to current realm [GnoVM] Implement a way to fetch PrevRealm Coins Jun 25, 2024
@leohhhn
Copy link
Contributor

leohhhn commented Jun 25, 2024

@thehowl This is an issue that is quite important. Does it make sense to include it into your std refactor?

After discussing internally, we definitely need a way to fetch PrevRealm Coins, as well as possibly remove OrigSend coins. @moul, how should we go about implementing this?

I see two possible options - either we create a std.PrevRealmCoins() function, or possibly add a Coins field into the Realm struct and make it accessible via getters - not sure how feasible the latter is.

We also need to keep in mind the way we send coins from one realm to another. Right now, this is banker.SendCoins(std.CurrentRealm().Addr(), otherRealmAddr, amt).

cc @Kouteki

@Kouteki
Copy link
Contributor

Kouteki commented Jun 25, 2024

The std refactor is part of the Test4 post launch milestone, same as this issue. We'll be tackling it after the launch.

@thehowl
Copy link
Member

thehowl commented Jun 25, 2024

@leohhhn thanks for bringing it up, it's actually something I remembered in the back of my head and that I introduced it in the design, though still TODO for now.

The idea is that you have the Deposits() function instead of OrigSend, with the idea of being usable also within called realms. I'm still unsure on how to make it work further down in the call stack (ie. should it contain everything that was sent to the realm in the same tx using banker? Or should there be a way to call a function with a deposit?). But I'm trying to keep it in mind as I proceed.

@thehowl thehowl mentioned this issue Jun 25, 2024
7 tasks
@thehowl thehowl changed the title [GnoVM] Implement a way to fetch PrevRealm Coins [GnoVM] OrigSend for realms called from another realm Jun 25, 2024
@Kouteki Kouteki added help wanted Want to contribute? We recommend these issues. and removed help wanted labels Oct 2, 2024
@thehowl
Copy link
Member

thehowl commented Oct 31, 2024

centralizing on #1475

@thehowl thehowl closed this as not planned Won't fix, can't repro, duplicate, stale Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Want to contribute? We recommend these issues. 📦 🤖 gnovm Issues or PRs gnovm related
Projects
Development

No branches or pull requests

5 participants