feat: ExecAsPkg

[albttx]

With @tbruyelle we continue @moul 's works on #335 to add an ExecAsPkg function.

This solution will be able to solve the issue on #634

Description

We add a new OpCall for setting the m *Machine context, this allow to update the context

How has this been tested?

see tests/files/zrealm_std6.gno

[moul]

I'm still uncertain, so I'd appreciate additional input. However, it appears that we have three primary options.

• std.ExecAsPkg
• std.UnsafeExecAsPkg
• unsafe.ExecAsPkg

I believe it is essential to reassess our understanding of safety and risk when drafting contracts. After all, isn't the default assumption that everything is potentially unsafe?

[albttx]

I havn't removed your comments.

But i think we should keep std.ExecAsPkg, it's a "normal feature" for a smart-contract engine to be able to do actions as the smart-contract addr

The use of the word Unsafe could scare contract writers, and make them think as a bad practice.

[piux2]

I havn't removed your comments.

But i think we should keep std.ExecAsPkg, it's a "normal feature" for a smart-contract engine to be able to do actions as the smart-contract addr

The use of the word Unsafe could scare contract writers, and make them think as a bad practice.

@albttx

Gnolang support pass function as parameters and interface callback. It does exactly what current std.ExecAsPkg's implementation tries to do, and is more flexible and safer.

Not sure if std.ExecAsPkg implementation does type checking on the function parameters.

pn.DefineNative("ExecAsPkg",
// TODO: rename UnsafeExecAsPkg?
gno.Flds( // params
"fn", gno.FuncT(nil, nil),
),

gno/stdlibs/stdlibs.go

Line 281 in d59c320

"fn", gno.FuncT(nil, nil),

On the other hand, Gnolang, as in go, supports the passing functions as parameters does type checking of the passed functions' parameter types

To solve #634, as you mentioned, you can modify GRC20 with interface callback or pass a callback function as a parameter to allow the airdrop contract to alter the state of your token contract. In addition, you can add a static allowed list or even try to verify interface types for approval.

That is more idiomatic in go programming, safe and auditable.

[piux2]

gno/tests/files/zrealm_std6.gno

Line 21 in d59c320

// g157y5v3k529jyzhjjz4fn49tzzhf4gess6v39xg

• Testing case is not robust. It exposes a safety issue for the contract to use std.ExecAsPkg. All three lines should have output the same address without knowing call path context has been changed.

• It sideloads the behavior of std.GetOrigCaller() and makes contracts hard to audit. The safety and security of smart contracts have very much to do with readability and auditability.

• More importantly, It makes checking std.GetOrigCaller() lose its purpose when we call std.ExecAsPkg(anyFunction).

Here is why:

std.GetOrigCaller() suppose to get the first caller's address of the entire call path. Usually, it is end users who initiate a function call on a smart contract. In Ethereum, that is an external owned account (EOA). In gno, we do not separate it since the main package can also be the OrigCaller.

No matter where std.GetOrigCaller() is called in the call path; it should always return the same address.

However, std.ExecAsPkg changes the context of the call path in the middle. It makes std.GetOrigCaller() returns the address of the contract that std.ExecAsPkg is called in.

gno/stdlibs/stdlibs.go

Line 308 in d59c320

m.PushOp(gno.OpSetContext)

// Push an alternate context with OrigCaller=OrigPkgAddr, this will
// affect the function call above
m.PushOp(gno.OpSetContext)
ctx := m.Context.(ExecContext)
ctx.OrigCaller = ctx.OrigPkgAddr

It makes checking std.GetOrigCaller() lose its purpose, when we call std.ExecAsPkg( any function ) in the same contract.

[tbruyelle]

We are still unsure why we need to add this useless value in the values' stack.

The thing is, if we don't, the stack is truncated by one value (when PopFrameReset is invoked). In that case, the truncated value is the context value we pushed earlier, and doOpSetContext cannot work properly without it.

We'll try to figure out why this value is truncated.

[tbruyelle]

After some investigation, in a normal context, this truncated value is the FuncValue that will be invoked. In this particular context (native call), the FuncValue comes from the native function parameter, so it's not in the values' stack.

Finally, instead of using OpCall, we should use OpPreCall that is the layer just above. This is done here
d59c320, and thankfully, we no longer need to push a placeholder value.

[moul]

We need more tests for this feature: multiple levels; reused func pointers; touching a variable outside of the scope; panic; probably more.

[moul]

Please share your thoughts on PR #683.

[jaekwon]

Agreed with piux, and as commented in other related PRs, we don't want this kind of dynamism.
There must be a pattern that uses only pure Go/Gno to achieve what we ultimately want.
Please see #566 (comment).
Also see #715 (review).