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

feat(gnoland): Define NoopMsg to facilitate the creation of sponsor transaction (gasless transaction) #2209

Open
wants to merge 24 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions gno.land/pkg/sdk/vm/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ func (vh vmHandler) Process(ctx sdk.Context, msg std.Msg) sdk.Result {
return vh.handleMsgCall(ctx, msg)
case MsgRun:
return vh.handleMsgRun(ctx, msg)
case MsgNoop:
return sdk.Result{}
default:
errMsg := fmt.Sprintf("unrecognized vm message type: %T", msg)
return abciResult(std.ErrUnknownRequest(errMsg))
Expand Down
32 changes: 32 additions & 0 deletions gno.land/pkg/sdk/vm/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package vm
import (
"testing"

"github.com/gnolang/gno/tm2/pkg/crypto"
"github.com/gnolang/gno/tm2/pkg/std"
"github.com/stretchr/testify/assert"
)

Expand Down Expand Up @@ -48,3 +50,33 @@ func Test_parseQueryEval_panic(t *testing.T) {
parseQueryEvalData("gno.land/r/demo/users")
})
}

func TestProcessNoopMsg(t *testing.T) {
// setup
env := setupTestEnv()
ctx := env.ctx
vmHandler := NewHandler(env.vmk)

addr := crypto.AddressFromPreimage([]byte("test1"))
msg := NewMsgNoop(addr)

res := vmHandler.Process(ctx, msg)
assert.Empty(t, res)
}

func TestProcessInvalidMsg(t *testing.T) {
// setup
env := setupTestEnv()
ctx := env.ctx
vmHandler := NewHandler(env.vmk)

type InvalidMsg struct {
std.Msg
}

msg := InvalidMsg{}

res := vmHandler.Process(ctx, msg)
assert.NotEmpty(t, res)
assert.Equal(t, res.Error, std.UnknownRequestError{})
}
41 changes: 41 additions & 0 deletions gno.land/pkg/sdk/vm/msgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -207,3 +207,44 @@
func (msg MsgRun) GetReceived() std.Coins {
return msg.Send
}

//----------------------------------------
// MsgNoop

// MsgNoop - executes nothing
type MsgNoop struct {
Caller crypto.Address `json:"caller" yaml:"caller"`
}

var _ std.Msg = MsgNoop{}

func NewMsgNoop(caller crypto.Address) MsgNoop {
return MsgNoop{
Caller: caller,
}
}

// Implements Msg.
func (msg MsgNoop) Route() string { return RouterKey }

Check warning on line 228 in gno.land/pkg/sdk/vm/msgs.go

View check run for this annotation

Codecov / codecov/patch

gno.land/pkg/sdk/vm/msgs.go#L228

Added line #L228 was not covered by tests

// Implements Msg.
func (msg MsgNoop) Type() string { return "no_op" }

Check warning on line 231 in gno.land/pkg/sdk/vm/msgs.go

View check run for this annotation

Codecov / codecov/patch

gno.land/pkg/sdk/vm/msgs.go#L231

Added line #L231 was not covered by tests

// Implements Msg.
func (msg MsgNoop) ValidateBasic() error {
if msg.Caller.IsZero() {
return std.ErrInvalidAddress("missing caller address")
}

return nil
}

// Implements Msg.
func (msg MsgNoop) GetSignBytes() []byte {
return std.MustSortJSON(amino.MustMarshalJSON(msg))

Check warning on line 244 in gno.land/pkg/sdk/vm/msgs.go

View check run for this annotation

Codecov / codecov/patch

gno.land/pkg/sdk/vm/msgs.go#L243-L244

Added lines #L243 - L244 were not covered by tests
}

// Implements Msg.
func (msg MsgNoop) GetSigners() []crypto.Address {
return []crypto.Address{msg.Caller}

Check warning on line 249 in gno.land/pkg/sdk/vm/msgs.go

View check run for this annotation

Codecov / codecov/patch

gno.land/pkg/sdk/vm/msgs.go#L248-L249

Added lines #L248 - L249 were not covered by tests
}
120 changes: 120 additions & 0 deletions gno.land/pkg/sdk/vm/msgs_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
package vm

import (
"testing"

"github.com/gnolang/gno/tm2/pkg/crypto"
"github.com/gnolang/gno/tm2/pkg/std"
"github.com/stretchr/testify/require"
)

func TestMsgAddPackage(t *testing.T) {
creator := crypto.AddressFromPreimage([]byte("addr1"))
pkgPath := "gno.land/r/namespace/test"
files := []*std.MemFile{
{
Name: "test.gno",
Body: `package test
func Echo() string {return "hello world"}`,
},
}

msg := NewMsgAddPackage(creator, pkgPath, files)

// Validate Basic
err := msg.ValidateBasic()
require.NoError(t, err)

// Check if package name is set correctly
require.Equal(t, msg.Package.Name, "test")

// Test invalid address
msg.Creator = crypto.Address{}
err = msg.ValidateBasic()
require.Error(t, err)

// Test invalid package path
msg.Creator = creator
msg.Package.Path = ""
err = msg.ValidateBasic()
require.Error(t, err)
}

func TestMsgCall(t *testing.T) {
caller := crypto.AddressFromPreimage([]byte("addr1"))
pkgPath := "gno.land/r/namespace/mypkg"
funcName := "MyFunction"
args := []string{"arg1", "arg2"}

msg := NewMsgCall(caller, std.Coins{}, pkgPath, funcName, args)

// Validate Basic
err := msg.ValidateBasic()
require.NoError(t, err)

// Test invalid caller address
msg.Caller = crypto.Address{}
err = msg.ValidateBasic()
require.Error(t, err)

// Test invalid package path
msg.Caller = caller
msg.PkgPath = ""
err = msg.ValidateBasic()
require.Error(t, err)

// Test invalid function name
msg.PkgPath = pkgPath
msg.Func = ""
err = msg.ValidateBasic()
require.Error(t, err)
}

func TestMsgRun(t *testing.T) {
caller := crypto.AddressFromPreimage([]byte("addr1"))
files := []*std.MemFile{
{
Name: "test.gno",
Body: `package main
func Echo() string {return "hello world"}`,
},
}

msg := NewMsgRun(caller, std.Coins{}, files)

// Validate Basic
err := msg.ValidateBasic()
require.NoError(t, err)

// Test invalid caller address
msg.Caller = crypto.Address{}
err = msg.ValidateBasic()
require.Error(t, err)

// Test invalid package name
files = []*std.MemFile{
{
Name: "test.gno",
Body: `package test
func Echo() string {return "hello world"}`,
},
}
require.Panics(t, func() {
NewMsgRun(caller, std.Coins{}, files)
})
}

func TestMsgNoop(t *testing.T) {
caller := crypto.AddressFromPreimage([]byte("addr1"))

msg := NewMsgNoop(caller)

// Validate Basic
err := msg.ValidateBasic()
require.NoError(t, err)

// Test invalid caller address
msg.Caller = crypto.Address{}
err = msg.ValidateBasic()
require.Error(t, err)
}
1 change: 1 addition & 0 deletions gno.land/pkg/sdk/vm/package.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ var Package = amino.RegisterPackage(amino.NewPackage(
MsgCall{}, "m_call",
MsgRun{}, "m_run",
MsgAddPackage{}, "m_addpkg", // TODO rename both to MsgAddPkg?
MsgNoop{}, "m_noop",

// errors
InvalidPkgPathError{}, "InvalidPkgPathError",
Expand Down
4 changes: 4 additions & 0 deletions gno.land/pkg/sdk/vm/vm.proto
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ message m_addpkg {
string deposit = 3;
}

message m_noop {
string caller = 1;
}

message InvalidPkgPathError {
}

Expand Down
12 changes: 10 additions & 2 deletions tm2/pkg/sdk/auth/ante.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,11 +139,19 @@ func NewAnteHandler(ak AccountKeeper, bank BankKeeperI, sigGasConsumer Signature
stdSigs := tx.GetSignatures()

for i := 0; i < len(stdSigs); i++ {
var isNewAccount bool
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
var isNewAccount bool
var isNewAccount = false

Copy link
Contributor Author

@linhpn99 linhpn99 Jul 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried and rolled back because the CI lint does not allow this type of declaration

// skip the fee payer, account is cached and fees were deducted already
if i != 0 {
signerAccs[i], res = GetSignerAcc(newCtx, ak, signerAddrs[i])
// only create new account when tx is a sponsor transaction
if !res.IsOK() {
return newCtx, res, true
if tx.IsSponsorTx() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think it should be a combination of:

  • is sponsored
  • acocunt_number == 0
  • pubkey not yet known

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great additions

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

However, this check

signerAccs[i], res = GetSignerAcc(newCtx, ak, signerAddrs[i])
// only create new account when tx is a sponsor transaction
if !res.IsOK() {
if tx.IsSponsorTx() {
isNewAccount = true
signerAccs[i] = ak.NewAccountWithAddress(newCtx, signerAddrs[i])
proves that account_number = 0 and public_key = nil

isNewAccount = true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sponsored tx is not only for new accounts

Copy link
Contributor Author

@linhpn99 linhpn99 Jul 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean this condition should be wrapped into a more general function for cases where a new account needs to be created, right?

Copy link
Contributor Author

@linhpn99 linhpn99 Jul 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@moul Actually, the logic here is: when the signer account is not found in the storage (the account receiving the sponsor has not existed on-chain before), we check if the transaction being processed is a sponsor transaction. Only then do we allow the creation of this new account on-chain instead of returning an error. This way, we can handle cases where the account does not exist but can still perform onchain actions through a sponsor transaction

signerAccs[i] = ak.NewAccountWithAddress(newCtx, signerAddrs[i])
signerAccs[i].SetPubKey(stdSigs[i].PubKey)
} else {
return newCtx, res, true
}
}
}

Expand All @@ -153,7 +161,7 @@ func NewAnteHandler(ak AccountKeeper, bank BankKeeperI, sigGasConsumer Signature
// No signatures are needed for genesis.
} else {
// Check signature
signBytes, err := GetSignBytes(newCtx.ChainID(), tx, sacc, isGenesis)
signBytes, err := GetSignBytes(newCtx.ChainID(), tx, sacc, isGenesis || isNewAccount)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need to check the signatures; maybe with a different algorithm, but we cannot completely skip signature verification

Copy link
Contributor Author

@linhpn99 linhpn99 Jul 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the logic here is able to verify all signatures within the transaction regardless of whether the account existed on-chain before or has just been created above

if err != nil {
return newCtx, res, true
}
Expand Down
45 changes: 45 additions & 0 deletions tm2/pkg/sdk/auth/ante_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -586,6 +586,51 @@ func TestAnteHandlerSetPubKey(t *testing.T) {
require.Nil(t, acc2.GetPubKey())
}

func TestAnteHandlerSponsorTx(t *testing.T) {
t.Parallel()

// setup
env := setupTestEnv()
anteHandler := NewAnteHandler(env.acck, env.bank, DefaultSigVerificationGasConsumer, defaultAnteOptions())
ctx := env.ctx

// keys and addresses
priv1, _, addr1 := tu.KeyTestPubAddr()
priv2, _, addr2 := tu.KeyTestPubAddr()

// Only create and set up account acc1 (addr1)
acc1 := env.acck.NewAccountWithAddress(ctx, addr1)
acc1.SetCoins(tu.NewTestCoins())
require.NoError(t, acc1.SetAccountNumber(0))
env.acck.SetAccount(ctx, acc1)

var tx std.Tx

// test good tx and set public key
noopMsg := tu.NewNoopMsg(addr1)
msg := tu.NewTestMsg(addr2)

msgs := []std.Msg{noopMsg, msg}

privs, accnums, seqs := []crypto.PrivKey{priv1, priv2}, []uint64{0, 0}, []uint64{0, 0}
fee := tu.NewTestFee()
tx = tu.NewTestTx(t, ctx.ChainID(), msgs, privs, accnums, seqs, fee)
checkValidTx(t, anteHandler, ctx, tx, false)

acc1 = env.acck.GetAccount(ctx, addr1)
require.Equal(t, acc1.GetPubKey(), priv1.PubKey())
require.Equal(t, acc1.GetAccountNumber(), uint64(0))
require.Equal(t, acc1.GetSequence(), uint64(1))

// Account for addr2 (acc2) has not been created yet.
// According to the new logic, if the transaction is a SponsorTx, a new account
// will be created for addr2 and its public key will be set.
acc2 := env.acck.GetAccount(ctx, addr2)
require.Equal(t, acc2.GetPubKey(), priv2.PubKey())
require.Equal(t, acc2.GetAccountNumber(), uint64(2))
require.Equal(t, acc1.GetSequence(), uint64(1))
}

func TestProcessPubKey(t *testing.T) {
t.Parallel()

Expand Down
1 change: 1 addition & 0 deletions tm2/pkg/sdk/testutils/package.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ var Package = amino.RegisterPackage(amino.NewPackage(
).WithDependencies().WithTypes(
// ...
&TestMsg{}, "TestMsg",
&NoopMsg{}, "NoopMsg",

// testmsgs.go
MsgCounter{},
Expand Down
28 changes: 28 additions & 0 deletions tm2/pkg/sdk/testutils/testutils.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,34 @@
return msg.Signers
}

// NoopMsg - executes nothing
type NoopMsg struct {
Caller crypto.Address
}

var _ std.Msg = &NoopMsg{}

func NewNoopMsg(caller crypto.Address) *NoopMsg {
return &NoopMsg{
Caller: caller,
}
}

// Implements Msg
func (msg NoopMsg) Route() string { return "TestMsg" }

Check warning on line 54 in tm2/pkg/sdk/testutils/testutils.go

View check run for this annotation

Codecov / codecov/patch

tm2/pkg/sdk/testutils/testutils.go#L54

Added line #L54 was not covered by tests
func (msg NoopMsg) Type() string { return "no_op" }
func (msg NoopMsg) GetSignBytes() []byte {
bz, err := amino.MarshalJSON(msg.Caller)
if err != nil {
panic(err)

Check warning on line 59 in tm2/pkg/sdk/testutils/testutils.go

View check run for this annotation

Codecov / codecov/patch

tm2/pkg/sdk/testutils/testutils.go#L56-L59

Added lines #L56 - L59 were not covered by tests
}
return std.MustSortJSON(bz)

Check warning on line 61 in tm2/pkg/sdk/testutils/testutils.go

View check run for this annotation

Codecov / codecov/patch

tm2/pkg/sdk/testutils/testutils.go#L61

Added line #L61 was not covered by tests
}
func (msg NoopMsg) ValidateBasic() error { return nil }

Check warning on line 63 in tm2/pkg/sdk/testutils/testutils.go

View check run for this annotation

Codecov / codecov/patch

tm2/pkg/sdk/testutils/testutils.go#L63

Added line #L63 was not covered by tests
func (msg NoopMsg) GetSigners() []crypto.Address {
return []crypto.Address{msg.Caller}
}

// ----------------------------------------
// Utility Methods

Expand Down
28 changes: 28 additions & 0 deletions tm2/pkg/std/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,34 @@
return nil
}

// IsSponsorTx checks if the transaction is a valid sponsor transaction
func (tx Tx) IsSponsorTx() bool {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, sponsortx is when:

  • first msg is noop
  • first msg signer != other ones

Copy link
Contributor Author

@linhpn99 linhpn99 Jul 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// At least two message in the transaction
if len(tx.Msgs) <= 1 {
return false
}

// The first message type is "no_op"
if tx.Msgs[0].Type() != "no_op" {
return false
}

// More than one signer
signers := tx.GetSigners()
if len(signers) <= 1 {
return false
}

// The first signer is different from all other signers
for i := 1; i < len(signers); i++ {
if signers[0] == signers[i] {
return false

Check warning on line 84 in tm2/pkg/std/tx.go

View check run for this annotation

Codecov / codecov/patch

tm2/pkg/std/tx.go#L84

Added line #L84 was not covered by tests
}
}

return true
}

// CountSubKeys counts the total number of keys for a multi-sig public key.
func CountSubKeys(pub crypto.PubKey) int {
v, ok := pub.(multisig.PubKeyMultisigThreshold)
Expand Down
Loading
Loading