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: add separator to rpc block_results to identify msg & response pair #2063

Conversation

r3v4s
Copy link
Contributor

@r3v4s r3v4s commented May 9, 2024

related to #2055

I'm opening this pr to get some ideas.

Currently, keeper does make response data with \n as separator to separate number of responses for single function.

for i, rtv := range rtvs {
res = res + rtv.String()
if i < len(rtvs)-1 {
res += "\n"
}
}

If single function returns 2 string, response may look like this

("1" string)
("2" string)

However as describe in #2055, response for single tx with multi-msg really doesn't separate response values.

So this pr adds \n\n as a separator for every function.

sample contract

package returns

func FuncNo() {}

func Func2() (string, uint64) {
	return "asd", 123
}

func Func3() (int64, string, uint64) {
	return -1, "hmm", 78978
}

call with multi-msg

await adena.DoContract({
  messages: [
    {
      "type": "/vm.m_call",
      "value": {
        "caller": "g1jg8mtutu9khhfwc4nxmuhcpftf0pajdhfvsqf5",
        "send": "",
        "pkg_path": "gno.land/r/r3v4/returns",
        "func": "FuncNo",
        "args": []
      }
    },
    {
      "type": "/vm.m_call",
      "value": {
        "caller": "g1jg8mtutu9khhfwc4nxmuhcpftf0pajdhfvsqf5",
        "send": "",
        "pkg_path": "gno.land/r/r3v4/returns",
        "func": "Func2",
        "args": []
      }
    },
    {
      "type": "/vm.m_call",
      "value": {
        "caller": "g1jg8mtutu9khhfwc4nxmuhcpftf0pajdhfvsqf5",
        "send": "",
        "pkg_path": "gno.land/r/r3v4/returns",
        "func": "FuncNo",
        "args": []
      }
    },
    {
      "type": "/vm.m_call",
      "value": {
        "caller": "g1jg8mtutu9khhfwc4nxmuhcpftf0pajdhfvsqf5",
        "send": "",
        "pkg_path": "gno.land/r/r3v4/returns",
        "func": "Func3",
        "args": []
      }
    },
    {
      "type": "/vm.m_call",
      "value": {
        "caller": "g1jg8mtutu9khhfwc4nxmuhcpftf0pajdhfvsqf5",
        "send": "",
        "pkg_path": "gno.land/r/r3v4/returns",
        "func": "FuncNo",
        "args": []
      }
    }
  ],
  gasFee: 1, 
  gasWanted: 2000000
});

block_results from rpc with b64 encoded

KCJhc2QiIHN0cmluZykKKDEyMyB1aW50NjQpKC0xIGludDY0KQooImhtbSIgc3RyaW5nKQooNzg5NzggdWludDY0KQ

b64 decoded

("asd" string)
(123 uint64)(-1 int64)
("hmm" string)
(78978 uint64)

two problem exists with above response

  1. It needs to do something about function that doesn't return any value
  2. It needs to divide results string for each function

in this pr response b64 encoded

CgooImFzZCIgc3RyaW5nKQooMTIzIHVpbnQ2NCkKCgoKKC0xIGludDY0KQooImhtbSIgc3RyaW5nKQooNzg5NzggdWludDY0KQoKCgo

decode b64 and divide by '\n\n'

image
From now, we know...

  1. tx had 5 messages
  2. first, third and fifth msg function doesn't return anything
  3. second msg function return 2 values
  4. fourth msg function return 3 values

@r3v4s r3v4s requested review from zivkovicmilos and leohhhn May 9, 2024 09:35
@github-actions github-actions bot added the 📦 ⛰️ gno.land Issues or PRs gno.land package related label May 9, 2024
Copy link

codecov bot commented May 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 54.19%. Comparing base (8057505) to head (054f323).
Report is 17 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2063      +/-   ##
==========================================
- Coverage   54.96%   54.19%   -0.77%     
==========================================
  Files         481      520      +39     
  Lines       67391    73195    +5804     
==========================================
+ Hits        37040    39668    +2628     
- Misses      27330    30300    +2970     
- Partials     3021     3227     +206     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@r3v4s
Copy link
Contributor Author

r3v4s commented May 9, 2024

@zivkovicmilos @leohhhn As we talked this pr does change rpc block_results response to identify msg & result pair.

But it doesn't cover custom return type such as type Blake struct.

Perhaps we can keep this pr open for historical, and find some better ways to resolve issue.

@r3v4s r3v4s marked this pull request as ready for review May 9, 2024 11:07
@r3v4s r3v4s requested review from moul, gfanton and thehowl as code owners May 9, 2024 11:07
@r3v4s r3v4s added help wanted investigating This behavior is still being tested out labels May 9, 2024
@Kouteki Kouteki added this to the 🏗4️⃣ test4.gno.land milestone May 10, 2024
Copy link
Member

@zivkovicmilos zivkovicmilos left a comment

Choose a reason for hiding this comment

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

I don't have a strong opinion on this, because I think that returning (<value> <type>) was not a good way to propagate realm responses in the first place to the user -- it makes it a nightmare for clients to:

  • prepare calls
  • parse responses

I honestly think the long term solution is #1842. However, I think this is a nice fix, so let's merge it 🚀

Copy link
Contributor

@ajnavarro ajnavarro left a comment

Choose a reason for hiding this comment

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

I cannot agree with this is closing #2055 , but if we remove that trigger from this PR we can merge it as a small quality of life from people trying to know what is the output from a specific version.

@r3v4s
Copy link
Contributor Author

r3v4s commented May 15, 2024

I cannot agree with this is closing #2055 , but if we remove that trigger from this PR we can merge it as a small quality of life from people trying to know what is the output from a specific version.

Good point, just changed to related rather than closes

@zivkovicmilos zivkovicmilos requested a review from ajnavarro May 15, 2024 09:33
@zivkovicmilos zivkovicmilos merged commit ccc6d5b into gnolang:master May 15, 2024
215 checks passed
leohhhn pushed a commit to leohhhn/gno that referenced this pull request May 21, 2024
… pair (gnolang#2063)

related to gnolang#2055 

I'm opening this pr to get some ideas.


Currently, keeper does make response data with `\n` as separator to
separate number of responses for single function.

https://github.com/gnolang/gno/blob/80575054429e07c221f1453104dd0ad29e33291c/gno.land/pkg/sdk/vm/keeper.go#L307-L312

If single function returns 2 string, response may look like this
```text
("1" string)
("2" string)
```

However as describe in gnolang#2055, response for single tx with multi-msg
really doesn't separate response values.

So this pr adds `\n\n` as a separator for every function.


### sample contract
```go
package returns

func FuncNo() {}

func Func2() (string, uint64) {
	return "asd", 123
}

func Func3() (int64, string, uint64) {
	return -1, "hmm", 78978
}

```

### call with multi-msg
```javascript
await adena.DoContract({
  messages: [
    {
      "type": "/vm.m_call",
      "value": {
        "caller": "g1jg8mtutu9khhfwc4nxmuhcpftf0pajdhfvsqf5",
        "send": "",
        "pkg_path": "gno.land/r/r3v4/returns",
        "func": "FuncNo",
        "args": []
      }
    },
    {
      "type": "/vm.m_call",
      "value": {
        "caller": "g1jg8mtutu9khhfwc4nxmuhcpftf0pajdhfvsqf5",
        "send": "",
        "pkg_path": "gno.land/r/r3v4/returns",
        "func": "Func2",
        "args": []
      }
    },
    {
      "type": "/vm.m_call",
      "value": {
        "caller": "g1jg8mtutu9khhfwc4nxmuhcpftf0pajdhfvsqf5",
        "send": "",
        "pkg_path": "gno.land/r/r3v4/returns",
        "func": "FuncNo",
        "args": []
      }
    },
    {
      "type": "/vm.m_call",
      "value": {
        "caller": "g1jg8mtutu9khhfwc4nxmuhcpftf0pajdhfvsqf5",
        "send": "",
        "pkg_path": "gno.land/r/r3v4/returns",
        "func": "Func3",
        "args": []
      }
    },
    {
      "type": "/vm.m_call",
      "value": {
        "caller": "g1jg8mtutu9khhfwc4nxmuhcpftf0pajdhfvsqf5",
        "send": "",
        "pkg_path": "gno.land/r/r3v4/returns",
        "func": "FuncNo",
        "args": []
      }
    }
  ],
  gasFee: 1, 
  gasWanted: 2000000
});
```
### block_results from rpc with b64 encoded
```
KCJhc2QiIHN0cmluZykKKDEyMyB1aW50NjQpKC0xIGludDY0KQooImhtbSIgc3RyaW5nKQooNzg5NzggdWludDY0KQ
```
### b64 decoded
```
("asd" string)
(123 uint64)(-1 int64)
("hmm" string)
(78978 uint64)
```
two problem exists with above response
1. It needs to do something about function that doesn't return any value
2. It needs to divide results string for each function


### in this pr response b64 encoded
```
CgooImFzZCIgc3RyaW5nKQooMTIzIHVpbnQ2NCkKCgoKKC0xIGludDY0KQooImhtbSIgc3RyaW5nKQooNzg5NzggdWludDY0KQoKCgo
```

### decode b64 and divide by '\n\n'

![image](https://github.com/gnolang/gno/assets/104744707/cf4ee0b8-a0d3-4fb8-aa59-68c814f8ef5b)
From now, we know...
1. tx had 5 messages
2. first, third and fifth msg function doesn't return anything
3. second msg function return 2 values
4. fourth msg function return 3 values
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
investigating This behavior is still being tested out 📦 ⛰️ gno.land Issues or PRs gno.land package related
Projects
Status: Done
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants