You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
When discussing PR #1591, @moul thinks that the PR is not an improvement, as he sees MsgRun first and foremost as a tool for humans and a tool that should have its closest equivalent to Go's main(), whereby unlike C it doesn't take arguments or return values, and that is what a Go developer expects.
This has a few major issues, in my opinion:
It creates a notion of an "output" on the blockchain, which before MsgRun was possible but never visible to any end user (it was just printed to the node stdout, and still is on MsgCall)
Consequently, it makes the printing format of println something that the users will depend upon, even though this should definitely not be depended on.
... especially because the buffer, effectively, can be written on by any function being called by Run, even maliciously.
ie. the caller of Run reads the first JSON object printed in the result, but when calling run which calls println(json.Marshal(myrlm.A())), myrlm.A is actually maliciously updated and now outputs an object of his own, hijacking the output.
Consequently, we convened that:
There should be a notion of "output" (ie. "stdout"), but this is meant for debugging purposes and not something that we should encourage users to rely on.
We can create consistency in our calls by allowing each of call, run, addpkg to have both an output and a return value.
There is a larger feature which can be adopted by MsgRun, and that is specifying an expression to be executed, similar to gno run -expr
ie maketx run -expr 'Hello()' -code 'package main; func Hello() int { return 42 }. In this case it makes sense that the user can specify a return value; however, if main() were still the only function allowed to run in maketx run and it did not support println + allowed return values, it would deviate too much from what programmers think as familiar with main().
The specific usecase of passing a realm output through a marshaler should not be seen as a priority; rather, creating ways for our client-node protocol to properly encode Gno values, be that through Amino, Protobuf, msgpack or JSON; ie. shift the responsibility of wire encoding from the realm to the "network layer".
When discussing PR #1591, @moul thinks that the PR is not an improvement, as he sees
MsgRun
first and foremost as a tool for humans and a tool that should have its closest equivalent to Go'smain()
, whereby unlike C it doesn't take arguments or return values, and that is what a Go developer expects.This has a few major issues, in my opinion:
println
something that the users will depend upon, even though this should definitely not be depended on.println(json.Marshal(myrlm.A()))
,myrlm.A
is actually maliciously updated and now outputs an object of his own, hijacking the output.Consequently, we convened that:
MsgRun
, and that is specifying an expression to be executed, similar togno run -expr
maketx run -expr 'Hello()' -code 'package main; func Hello() int { return 42 }
. In this case it makes sense that the user can specify a return value; however, ifmain()
were still the only function allowed to run inmaketx run
and it did not support println + allowed return values, it would deviate too much from what programmers think as familiar withmain()
.Originally posted by @thehowl in #1591 (comment)
The text was updated successfully, but these errors were encountered: