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

goal: Better formatting in goal clerk simulate #5259

Merged

Conversation

jasonpaulos
Copy link
Member

Summary

This PR uses the new REST client feature from #5253 to allow the client to get msgpack simulation responses. With this, the client can decode to the more precise v2.PreEncodedSimulateResponse instead of model.SimulateResponse.

The only difference between these two types is that model.SimulateResponse uses map[string]interface{} to represent transactions, while v2.PreEncodedSimulateResponse uses transactions.SignedTxn. This results in a minor problem, since our JSON decoder will decode integers as float64 by default, and this makes responses look strange when goal clerk simulate re-encodes the JSON response.

For example, this response from goal clerk simulate adds .0 after integers in the signed transaction:

{
  "last-round": 921,
  "txn-groups": [
    {
      "app-budget-added": 700,
      "app-budget-consumed": 50,
      "failed-at": [
        1
      ],
      "failure-message": "transaction JZ75D2OJEKIUIKLCS2BIRI4JOCMKU5YDP6YADIGZSDZGB7ZZHV4Q: logic eval error: assert failed pc=285. Details: pc=285, opcodes=txnas Accounts\n==\nassert\n",
      "txn-results": [
        {
          "missing-signature": true,
          "txn-result": {
            "pool-error": "",
            "txn": {
              "txn": {
                "amt": 20000000.0,
                "fee": 1000.0,
                "fv": 903.0,
                "gen": "Default Network Template-v1",
                "gh": "9YlKpP21uyjwiWP9dW8qdgyrtXgj+11tFimt2zgzyL0=",
                "grp": "4vqZ9bYDtPUWspTMWjyfFDywcimjOp9vdRvOr+lD6Mw=",
                "lv": 1903.0,
                "note": "LdvdxHRNj64=",
                "rcv": "WCS6TVPJRBSARHLN2326LRU5BYVJZUKI2VJ53CAWKYYHDE455ZGKANWMGM",
                "snd": "KJXS3MA3OEBUAXDU4P6VTHFYEUQUKJYYGD4I2ORDFNZBQI7OOIOYBRH74A",
                "type": "pay"
              }
            }
          }
        },
        {
          "app-budget-consumed": 50,
          "missing-signature": true,
          "txn-result": {
            "pool-error": "",
            "txn": {
              "txn": {
                "apaa": [
                  "J7vOsg==",
                  "AA=="
                ],
                "apid": 1.0,
                "fee": 1000.0,
                "fv": 919.0,
                "gh": "9YlKpP21uyjwiWP9dW8qdgyrtXgj+11tFimt2zgzyL0=",
                "grp": "4vqZ9bYDtPUWspTMWjyfFDywcimjOp9vdRvOr+lD6Mw=",
                "lv": 1919.0,
                "note": "P2NnKKJRzNI=",
                "snd": "ILAZQ3YQUQKGMXTXICRP6422NFVTKLPNQZKHMZH4RSTDO6C7K6HH5VQBRY",
                "type": "appl"
              }
            }
          }
        }
      ]
    }
  ],
  "version": 1,
  "would-succeed": false
}

Test Plan

Add additional test in test/e2e-go/restAPI/restClient_test.go

@jasonpaulos jasonpaulos changed the title goal: Report better simulation object in goal clerk simulate goal: Better formatting in goal clerk simulate Apr 5, 2023
@ahangsu ahangsu self-requested a review April 5, 2023 18:59
@codecov
Copy link

codecov bot commented Apr 5, 2023

Codecov Report

Merging #5259 (4a85c20) into master (f4f5ec6) will increase coverage by 0.01%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master    #5259      +/-   ##
==========================================
+ Coverage   53.78%   53.80%   +0.01%     
==========================================
  Files         450      450              
  Lines       56201    56208       +7     
==========================================
+ Hits        30226    30240      +14     
+ Misses      23626    23625       -1     
+ Partials     2349     2343       -6     
Impacted Files Coverage Δ
cmd/goal/clerk.go 9.10% <0.00%> (ø)
daemon/algod/api/server/v2/handlers.go 0.83% <ø> (ø)
daemon/algod/api/server/v2/utils.go 10.91% <0.00%> (ø)
libgoal/libgoal.go 2.48% <0.00%> (-0.03%) ⬇️

... and 8 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@ahangsu ahangsu left a comment

Choose a reason for hiding this comment

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

generally lgtm on first pass, have a question on restClient implementation

daemon/algod/api/client/restClient.go Show resolved Hide resolved
bbroder-algo
bbroder-algo previously approved these changes Apr 5, 2023
Copy link
Contributor

@bbroder-algo bbroder-algo left a comment

Choose a reason for hiding this comment

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

I'm good either way

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants