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

Make banker.SendCoins take variadic Coin argument #1676

Closed
leohhhn opened this issue Feb 22, 2024 · 4 comments
Closed

Make banker.SendCoins take variadic Coin argument #1676

leohhhn opened this issue Feb 22, 2024 · 4 comments
Assignees

Comments

@leohhhn
Copy link
Contributor

leohhhn commented Feb 22, 2024

Description

As the title says. Instead of having to build a slice each time you want to send coins, I propose we change the banker API to take in a variadic Coin argument.

Before:
func (ba bankAdapter) SendCoins(from, to Address, amt Coins)

After:
func (ba bankAdapter) SendCoins(from, to Address, amts ...Coin)

Here is the code.

@leohhhn leohhhn changed the title Make banker.SendCoins take variadic Coin argument Make banker.SendCoins take variadic Coin argument Feb 22, 2024
@thehowl
Copy link
Member

thehowl commented Feb 23, 2024

Note, the Coin and Coins type are missing many useful methods and constructors from their original form in tm2/pkg/std: https://github.com/gnolang/gno/blob/master/tm2/pkg/std/coin.go

IMO a better approach is to add better constructors for the types; using the Coins type is useful as it has useful methods to do logic on coins

@leohhhn
Copy link
Contributor Author

leohhhn commented Feb 26, 2024

I was more referring to the fact that when you want to send a single coin with the banker, you always need to pack it inside the Coins struct, which can get a bit annoying. Example:

realmAddr := std.CurrentRealm().Addr()
coinToSend := banker.GetCoins(realmAddr).AmountOf("ugnot")
banker.SendCoins(realmAddr, to, std.Coins{{ "ugnot", coinToSend }})

----

// bit nicer if:
banker.SendCoins(realmAddr, to, std.Coin{"ugnot", coinToSend})

// and can do
coins := std.Coins{...}
banker.SendCoins(realmAddr, to, coins...)

The other option is what you are suggesting, and that is adding the NewCoins constructor, which takes in a variadic Coin arg. 🤷

BTW, I am working on a small PR to introduce some more functionality to Coins/Coin in Gno by porting some stuff from coin.go.

@thehowl
Copy link
Member

thehowl commented Feb 27, 2024

Yeah, I get you, what I'm saying is that Coins is a useful type and maybe we should tackle the underlying issue here by making its constructors useful.

@leohhhn
Copy link
Contributor Author

leohhhn commented May 28, 2024

Solution for this was to implement a variadic constructor for the Coins type. Implemented in #2104

@leohhhn leohhhn closed this as completed May 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

No branches or pull requests

2 participants