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

[WIP]feat: Protocol to support contract-contract interaction #473

Draft
wants to merge 21 commits into
base: master
Choose a base branch
from

Conversation

ltzmaxwell
Copy link
Contributor

Description

  1. it defines a standard result format from VM;
  2. it defines a new msg type: innerMsg, which allows you to make an inter-contract call and callback.

@ltzmaxwell ltzmaxwell requested a review from a team as a code owner January 21, 2023 12:05
@moul
Copy link
Member

moul commented Jan 21, 2023

Thank you @ltzmaxwell. Can you add a pure .gno test too?

@piux2
Copy link
Contributor

piux2 commented Jan 23, 2023

Description

  1. it defines a standard result format from VM;
  2. it defines a new msg type: innerMsg, which allows you to make an inter-contract call and callback.

Thank you for your contribution.

Can you help us understand what we are trying to achieve with innerMsg, std.Result and std.Response?

  1. The idiomatic way in gno to call another package is to import a package and call the export function. Why will we need to construct an innerMsg to do it?

     []std.InnerMsg{callMsg0, callMsg1}
    

    examples/gno.land/r/demo/x/inner_call/master/master.gno:76

  2. For the callback, the idiomatic way is to use an interface type or pass a function as a parameter. Both callback methods are supported for gno realm packages already. We can define any function signatures for the callback functions. However, the “Callback” function name is hard-coded in VM. What is the purpose of the dispatcher?

    callBackMsg := InnerMsg{
     :
             Func:    "Callback",
         :
    }
    

    pkgs/sdk/vm/dispatcher.go:86

  3. the VM abstracts away the message response and request from underlining transactions. So why will we need to create and expose the std. Request{} and Response{} in the gno contract and sending msg explicitly in Gno package?

     std.GnoResult(res)
    

    examples/gno.land/r/demo/x/inner_call/master/master.gno:52

  4. Function call between realm packages is type checked by the VM, std.Result and std.Response prevent the type check on the parameters and results between function calls. What is the reasoning behind it?

     func Success(s string) *std.Result {
    

    examples/gno.land/r/demo/x/inner_call/master/master.gno:28

     callMsg1 := std.InnerMsg{
     :
     Func:    "Success",
     Args:    []string{"butterworth"},
     :
     }
    

    examples/gno.land/r/demo/x/inner_call/master/master.gno:37

@moul
Copy link
Member

moul commented Jan 24, 2023

Spoiler: I didn't review your implementation yet, but will do soon.

@piux2, one of the exciting points with contract-contract messages is if it can create a new sdk.Context where the caller is the other contract. So we can start making things like contract-based multisigs.
It's related with #335, and a little bit with #402.

@moul
Copy link
Member

moul commented Jan 24, 2023

Another point could be to support new kind of update proxies; where you can dynamically change the "import path" by calling a SetAddr(newContractPath) helper.

@anarcher
Copy link
Contributor

Another point could be to support new kind of update proxies; where you can dynamically change the "import path" by calling a SetAddr(newContractPath) helper.

My idea may not have much to do with this issue, but I thought about adding std.Call.

res := std.Call("gno.land/r/demo/user","GetUser","username")

Exceptionally, it seems that there are cases where dynamic imports are needed in contracts, rather than static imports. (e.g. GRC20 token management app?)

@giansalex
Copy link
Contributor

I was thinking:

var nft grc721.IGRC721 = std.Import("gno.land/r/demo/nft")
nft.TransferFrom(...)

@anarcher
Copy link
Contributor

I was thinking:

var nft grc721.IGRC721 = std.Import("gno.land/r/demo/nft")
nft.TransferFrom(...)

I also thought of std.Import. :-D. However, IMHO, it didn't seem appropriate to treat package itself as a value in go (and gno?). Package itself can't be interface.

@piux2
Copy link
Contributor

piux2 commented Jan 24, 2023

Spoiler: I didn't review your implementation yet, but will do soon.

@piux2, one of the exciting points with contract-contract messages is if it can create a new sdk.Context where the caller is the other contract. So we can start making things like contract-based multisigs. It's related with #335, and a little bit with #402.

The idiomatic way to call between contracts is to Import the realm package and call the method to access the package state stored in the package variables. The caller in the path can be accessed through std.GetCallerAt(i),

regarding others

  1. we don't need to implement contract-based multi-sig; people do that in Solidity because ETH does not support threshold signature in the Lay1 protocol. To approve DAO spending between contracts, the voting quorum configuration and approval process will take care of it. There is no signature and key signing involved between contract calls.

  2. For exp: add std.ExecAsPkg #335, it does not apply to gno, or we can say the functionality has already been taken care of by MsgCall.

Fundamentally Gno VM is modeled differently than EVM.
In solidity, call(txdata) txdata is byte code. EVM stores and execute the byte code.

Fundamentally Gno VM is modeled differently than EVM.

In solidity, call(txdata) txdata is byte code. EVM stores and execute the byte code. The solidity contract itself is not stored or executed upon on chain. We should avoid low-level calls .call() whenever possible when executing another contract function, as it bypasses type checking, function existence check, and argument packing.

GNO VM is a go syntax interpreter, it does not store and execute bytecode, it stores elements of parsed .gno files as type, object, and blocknodes, and VM executes these parse elements (type, value, expression, variable, and such). In GNO, the real "raw txdata" in VM are these parsed objects encoded in amino+ proto

  1. feat: allow current pkg access to bank #402, we can optimize GetCallerAt to apply to broad use-case including feat: allow current pkg access to bank #402. The current fix only works for Bank instances.

@piux2
Copy link
Contributor

piux2 commented Jan 25, 2023

Another point could be to support a new kind of update proxies; where you can dynamically change the "import path" by calling a SetAddr(newContractPath) helper.

This is worth some discussion and lays out the pros, cons, and trade-offs

One valid use case for dynamic importing is the proxy contract upgrade. However, the proxy contract upgrade is a controversial practice. Basically, it trades off security, trust, and complexity with certain flexibility.

What are other use cases we have to use dynamic package loading?

import "std"

func main(){
// define msg here, marshal it and print to stdout, to trigger a msg call
Copy link
Contributor

Choose a reason for hiding this comment

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

this this is how the system works I think could be better documented somewhere.

@jaekwon
Copy link
Contributor

jaekwon commented Jan 25, 2023

Thank you for the PR!
As this is a significant feature, I do want to fully understand it.
Are the results passed on as args to the next message? Are all the args strings?
A design doc would be helpful, and maybe for future PRs we should start there (that is, significant changes like this don't need to be fully implemented before we discuss the design).
Does it bypass type-checking?
What are the security issues that may arise?
These are some general questions, but I haven't had time to fully grok it yet.

A minimal use-case example may be helpful for me and others.

@ltzmaxwell
Copy link
Contributor Author

ltzmaxwell commented Jan 27, 2023

Thanks guys, for your questions and discussions, it has given me a lot of inspiration, many of which I did not expect when I started this work. Really appreciate!

Motivation

First, I was looking for a way to dynamically call another contract from a host contract. That would be std.Call, std.Import or something like that.

Then I realized that a messaging mechanism might be a better alternative, so some experimentation is done.

General Flow:

    Forward  Flow:    Contract A -> Contract B -> Contract C
    Backward Flow:    Contract A <- Contract B <- Contract C

This is fairly straightforward, while the challenge is to keep the forward&backward flow atomic, consistent, and robust.

InnerMsg:

InnerMsg is a wrapper of std.Msg (now only MsgCall supported), with an additional field MsgId, which is used for result handling of the host contract.

std.Result

InnerMsg is wrapped in std.Result as well as some other fields like: data, event, log, etc. std.Result is defined in stdlibs, converted to Go type for further handling: to return a sdk.Result if no InnerMsgs returned, or to feed the dispatcher.

Dispatcher:

The dispatcher works as a coordinator, it converts innerMsg to a standard MsgCall, feeds the vm message handler, then does a callback to the host contract if the called contract returns success, or reverts if it returns error.

Test & Simulate:

Since the message mechanism involves some preprocessing, such as message validation and dispathing, the test should include this as well.

A simulator is a tool to simulate an MsgCall for its entire lifecycle. The InjectMsgs method in simulation.go is the same as the runMsgs in baseapp.go, except that it's exported. The simulator is bridged to the filetest system to make a message simulation.

The way we write the test may change a bit, what we need to do is:
Define messages, marshal them and print to stdout, it is caught by a newly introduced directive: Await, then unmarshaled to construct standard MsgCall, and feed the simulator, start the loop.

Potential problems and possibilities:

There may be some potential problems, like inifinte loops, permission issues, etc., but I think they would be solvable.

While some another exiting possibilities may also arise, since interoperability(dynamically) is provided, some idiom and paradigms of development work will change as a result, bringing more surprises and possibilities.

Responding to concerns:

Does it bypass type checking?

Msgs are injected into the VM keeper rather than a low-level call, it's some standalone MsgCalls chaining together, so I think is not bypassing typecheck.

Are all args string?

Yes. InnerMsg is converted to a std.Msg before the loop begin.

@ltzmaxwell
Copy link
Contributor Author

hi, @moul, are these potentially related?

// "vm" -- starts an in-memory chain that can be interacted with?


Test & Simulate:

Since the message mechanism involves some preprocessing, such as message validation and dispathing, the test should include this as well.

A simulator is a tool to simulate an MsgCall for its entire lifecycle. The InjectMsgs method in simulation.go is the same as the runMsgs in baseapp.go, except that it's exported. The simulator is bridged to the filetest system to make a message simulation.

The way we write the test may change a bit, what we need to do is: Define messages, marshal them and print to stdout, it is caught by a newly introduced directive: Await, then unmarshaled to construct standard MsgCall, and feed the simulator, start the loop.

@moul
Copy link
Member

moul commented Jan 29, 2023

hi, @moul, are these potentially related?

// "vm" -- starts an in-memory chain that can be interacted with?

This comment was not based on a planned feature, but was an invitation to discuss if something could make sense.

Maybe we can imagine something different to test your workflow, like having new testing helpers/drivers.

Maybe we can also define a shareable format made for running a suite of TXs against a localnet of a production network.

I'll keep thinking.

@moul
Copy link
Member

moul commented Mar 1, 2023

We need more input on this one.
cc @loicttn @mvertes @QuentinPerez @aimxhaisse


My current feeling is that we should have at least two methods to make inter-contract calls:

  1. idiomatic one, preserving typing, easy to read -- I imagine helpers patch the stack, so a requested contract believes that the calling contract is the requesting user. It would allow advanced patterns like contract-based wallets, multi-sigs, etc. That's what I started on exp: add std.ExecAsPkg #335.
  2. this one adds advanced use cases like being able to call a not-yet-published package or a package defined as an updatable string, etc. Naively, I would try to use an RPC/amino/protobuf style of API, a.k.a., considering that the remote contract is a micro-service hosted elsewhere. So the good way to do this is to support marshaling/unmarshalling against structs, playing with interfaces, and generating code where it makes sense.

@piux2
Copy link
Contributor

piux2 commented Mar 3, 2023

General Flow:

    Forward  Flow:    Contract A -> Contract B -> Contract C
    Backward Flow:    Contract A <- Contract B <- Contract C

This is fairly straightforward, while the challenge is to keep the forward&backward flow atomic, consistent, and robust.

These contracts are not individual services when they call each other or in the call back flow.

The entire call or call back flow is atomic, since GNO VM will put each functions called by the contracts on stack first and executed by the GNO VM in FILO fashion. In the end, the package variable state are updated and committed in the state store. if there is any exception, the VM will panic and the final state will not be committed.

The GNO VM even handle's stack over flow if there are recursive calls. It does slow down the VM though, but GNO VM will recover. No contact variable states are updated either.

@ltzmaxwell
Copy link
Contributor Author

Not sure but came up with an idea, is it possible to build a unified model, that a standard msg could be routed to another contract, or another chain(IBC)?

@piux2
Copy link
Contributor

piux2 commented Mar 18, 2023

Not sure, but came up with an idea. Is it possible to build a unified model that a standard msg could be routed to another contract or another chain(IBC)?

Having a way to call contracts between different chains linked by IBC is a great idea!

The IBC channel is an abstraction on top of the message broadcast to the network. There is no actual point-to-point IBC message call directly between micro-services or chains. Instead, the IBC message is broadcast to the entire network of chain A as many other messages; the relayer scans the state of the local chain, sees the message included with the IBC information to chain B, and then relays it to the destination chain B.

In other words, IBC is an abstraction modeled as a TCP connection. However, the underline implementation is more like a distributed database sync facilitated by IBC relayers.

High-level proposal

Due to the nature of IBC, as for call contracts on a destination chain connected through IBC, we will need to define a global realm URI format included in the IBC message, for example, ibc_chain_id+local realm URI. We do not need to build a proxy or anything to facilitate the message transfer. The message is delivered as part of the IBC message as follows:

  • The call message, including the global realm URI ( chain_id+local realm), is broadcasted to network A as an IBC message.
  • Call message included in the network A's state after Tendermint nodes reach the consensus.
  • A relayer relays the call message A to the destination chain B after
  • The destination chain IBC module processes the IBC message and routes the message to the GVM module on the destination chain.
  • The destination GVM module looks at the call message, validates the global realm URI, and checks whether it should and could process it. (the global realm URI validation could be done in the IBC module before it routes the message to the GVM module. However, such behavior will not be compatible with the standard IBC)
  • GVM executes the message.

Atomic function calls cross chains

This will only be possible if we coordinate locking and releasing the contracts state on both chains between calls. However, there is no instant and real-time finality guaranteed cross chains. We need strong use cases to justify the complexity introduced and the trade-off between usability and functionality.

Call back between contracts cross chains

Application developers can implement either interface callback or pass a function as parameters to achieve cross-chain contract calls using the global realm URI.

We must decide whether we allow callback and recursive calls between cross-chain contracts.

  • Callback without atomicity assumption can be achieved without introducing too much complexity. However, that assumption burdens the dev. It is also an issue people might solve creatively on the contract level. It could introduce security issues that are hard to foresee and understand later.

  • Recursive calls are challenging since it involves maintaining global stacks, state cross chains, and handling atomic function calls cross chain. Therefore, we need strong use cases to justify the complexity introduced and the trade-off between usability and functionality.

@ltzmaxwell
Copy link
Contributor Author

Thank you @piux2 for the comment, it's amazing.

@ltzmaxwell
Copy link
Contributor Author

ltzmaxwell commented Apr 12, 2023

Update:

In the V1 implementation, an InnerMsg is introduced, which closely resembles a MsgCall. It is embedded within a VMResult, which is then returned to the VM keeper. Subsequently, the VM keeper constructs a MsgCall targeting the desired contract and initiates a callback to the calling contract. The contract side pseudocode appears as follows:

   func Success(s string) *std.Result {
		callMsg0 := std.InnerMsg{
			MsgID    :  0,
			Caller:  std.GetOrigCaller(),
			PkgPath: "gno.land/r/demo/x/inner_call/worker1",
			Func:    "Success",
			Args:    []string{"maxwell"},
		}

		res := std.NewResponse().
		    WithInnerMsgs([]std.InnerMsg{callMsg0}).
			WithLog("log from master").
			WithInfo("info from master")

		return std.GnoResult(res)
    }

   func Callback(arg0, arg1 string) {
       // handle callback
   }

Alternatively, a more RPC-style approach can be introduced using std.Call. This method accepts requests from the contract side and sends them to the VM keeper through a channel, which is then processed. The code looks like this:

   package hello

	import "std"

	func SayHello() {

		call := &std.CallMsg{
		    PkgPath: "gno.land/r/demo/greet",
		    Fn: "Greet",     
		    Args: []string{"hey"}, 
		}

		callback := &std.CallMsg{
		    PkgPath: "gno.land/r/demo/hello",
		    Fn: "Reply",     
		}

		std.Call(call, callback)

		println("after call, the call will be handled, wating for callback")
	}

    // callback
	func Reply(r string){
		println("receive callback: ", r)
	}

However, improvements can be made to achieve a more idiomatic implementation, such as using client.XXX(req) instead of std.Call. Maybe introducing a proto buffer schema and code generation to build types and bindings could be helpful, with std.Call wrapped within.

Moreover, regardless of the style chosen, both have the potential to connect with IBC since messages can flow to another contract, either within the same VM or on a different chain, as long as the necessary rules are established.

So, the questions is which style is better? not limited to this question, any suggestions, corrections, and clarifications will be helpful, Thank you!

@moul

@@ -141,6 +141,61 @@ func InjectPackage(store gno.Store, pn *gno.PackageNode) {
m.PushValue(res0)
},
)
// TODO: IBC handshake process
// this is called mannuly, maybe should have some modifier for the contract,like `IBC`,
// so the IBC channel is initialized during deploying, and can be used more efficiently
Copy link
Member

Choose a reason for hiding this comment

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

The IBC channel should be chain-managed, with a consistent queue creation method initiated by the contract or the chain.

// )
// m.PushValue(res0)

go SendData(msg)
Copy link
Member

Choose a reason for hiding this comment

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

Do you plan to have something like this?

Suggested change
go SendData(msg)
errChan := chan(error)
go SendData(msg, errChan)
recvChan := chan(string)
select {
case recv := <-recvChan: // ...
case timeout: // ...
}

Else, I'm curious about what's your plan?

Copy link
Contributor Author

@ltzmaxwell ltzmaxwell Apr 14, 2023

Choose a reason for hiding this comment

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

We have two ways to achieve this, like using the channel to send and receive msg, with timeout set properly, this is more like a synchronous call, it will block until the call is finished.

Another way is using a callback, that the VM keeper initiate a callback to the caller after the target call is finished, this is more like an asynchronous call, but brings complexity compared to the first one.

Both have pros and cons, what do you think.

Copy link
Member

Choose a reason for hiding this comment

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

I'm considering eliminating the goroutine since we could have events in a queue without waiting.

Can you begin implementation as per your preference, and we can discuss it later? Perhaps I can propose an alternative solution.

Copy link
Contributor Author

@ltzmaxwell ltzmaxwell May 5, 2023

Choose a reason for hiding this comment

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

A prototype, here is a brief introduction:

Contract:

The caller contract is something like this, it firstly encodes the parameters, invoke a std.Await(exactly name to be determined, name it await since it's synchronous call"), and wait for the result.

 package hello

 import "std"

 func Hello() string {

 	chainID := std.GetChainID()
 	call := std.EncodeMsgCall(chainID, "gno.land/r/demo/x/calls/await/greet", "Greet", []string{"hey"})

 	r, err := std.Await(call)
 	println("done ")
 	println("r is :", r)
 	println("errMsg is: ", err)

 	return "hello" + r
}

The encoder is very rough, just a simple stringification like:

package std

 // TODO: a better encoder
 func EncodeMsgCall(ChainID, PkgPath, Fn string, args []string) string {
 	var as string
 	for _, a := range args {
 		as += a
 	}
 	return ChainID + "#" + PkgPath + "#" + Fn + "#" + as
 }

Note: the ChainID is used temperarily to identify whether this is a in-VM call or IBC call.

Stdlib:

The stringified call is decoded into a GnoMsg, which mostly resembles MsgCall but with additionally field of ChainID , and field Response, that is a chan string as a callback to retrieve result.

pn.DefineNative("Await",
 			gno.Flds( // params
 				"call", "string",
 			),
 			gno.Flds( // results
 				"bz", "string",
 				"err", "string",
 			),
 			func(m *gno.Machine) {
 				// println("std.Send")
 				arg0 := m.LastBlock().GetParams1()
 				call := arg0.TV.GetString()
 				// println("call: ", call)

 				gnoMsg, err := vmh.DecodeMsg(call)
 				if err != nil {
 					panic("parameter invalid")
 				}
 				resQueue := make(chan string, 1)
 				gnoMsg.Response = resQueue

 				// send msg
 				vmkeeper.DispatchInternalMsg(gnoMsg)

 				// XXX: how this determined, since calls will accumulate
 				// should have an estimation like gas estimation?
 				timeout := 3 * time.Second
 				println("block, waiting for result...")

 				// TODO: err handling
 				var result string
 				select {
 				case result = <-resQueue:
 					println("callback in recvMsg: ", result)
 				case <-time.After(timeout):
 					panic("time out")
 					// case err = <- errQueue:
 				}

 				// TODO: return err to contract
 				var errMsg string
 				if err != nil {
 					errMsg = err.Error()
 				}

 				res := gno.Go2GnoValue(
 					m.Alloc,
 					m.Store,
 					reflect.ValueOf(result),
 				)
 				m.PushValue(res)
 				m.PushValue(typedString(gno.StringValue(errMsg)))
 			},
 		)

VMKeeper:

A eventLoop routine is always listening new event from contract, and then handle it.

func (vmk *VMKeeper) startEventLoop() {
	for {
		select {
		case msg := <-vmk.internalMsgQueue:
			go vmk.HandleMsg(msg)
		}
	}
}

The handler calls the callee contract in-VM or through IBC. We don't have IBC now, so using a channel to simulate the IBC loop.

func (vmk *VMKeeper) HandleMsg(msg vmh.GnoMsg) {
	println("------HandleMsg, routine spawned ------")
	// prepare call
	msgCall, isLocal, response, err := vmk.preprocessMessage(msg)
	if err != nil {
		panic(err.Error())
	}
	// do the call
	if isLocal {
		println("in VM call")
		println("msgCall: ", msgCall.Caller.String(), msgCall.PkgPath, msgCall.Func, msgCall.Args[0])

		r, err := vmk.Call(vmk.ctx, msgCall)
		println("call finished, res: ", r)
		// have an return
		if err == nil {
			response <- r
		}
	} else { // IBC call
		println("IBC call")
		// send IBC packet, waiting for OnRecv
		vmk.ibcResponseQueue = response
		vmk.SendIBCMsg(vmk.ctx, msgCall)
	}
}

So it's a basic prototype, wonder if it's the right path?

To be determined and done:

  1. Sync or Async? if it's a cascade call, like a->b->c, the return value would be useful so a synchronous call will be needed, but if it's something with no strong dependencies, use Async call with events emitted would be enough.
  2. The proper name ;
  3. the std.GetXXXCaller related work. this may relate to some security issues, should get whole call graph to solve the impersonate issues?
  4. gas management;
  5. better encoder and decoder?
  6. more use case and test.

"github.com/gnolang/gno/stdlibs"
)

type Wrapper struct {
Copy link
Contributor

@jaekwon jaekwon Apr 12, 2023

Choose a reason for hiding this comment

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

Is there a more specific name we can use?
A CallContext?
Is there a better word than wrapper or context?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure this structure is really making the program easier to understand.

Copy link
Contributor

Choose a reason for hiding this comment

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

I do agree that we want some kind of refactoring but I think it would be better to revert the Wrapper stuff here and to make a separate PR later with perhaps a different refactor.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, absolutely, I'm splitting it out to another PR, #718.

@r3v4s
Copy link
Contributor

r3v4s commented Jun 19, 2023

@ltzmaxwell do you think pr needs bit of modification due to #667 merged?

@moul moul added this to the 🌟 main.gno.land (wanted) milestone Sep 8, 2023
@github-actions github-actions bot added 🧾 package/realm Tag used for new Realms or Packages. 📦 🤖 gnovm Issues or PRs gnovm related 📦 🌐 tendermint v2 Issues or PRs tm2 related 📦 ⛰️ gno.land Issues or PRs gno.land package related labels Oct 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📦 🌐 tendermint v2 Issues or PRs tm2 related 📦 ⛰️ gno.land Issues or PRs gno.land package related 📦 🤖 gnovm Issues or PRs gnovm related 🧾 package/realm Tag used for new Realms or Packages.
Projects
Status: To triage
Status: No status
Status: 🌟 Wanted for Launch
Development

Successfully merging this pull request may close these issues.

8 participants