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(cmd): Add tx show subcommand for pretty-printing transactions #27

Merged
merged 1 commit into from
Feb 17, 2023

Conversation

kostko
Copy link
Member

@kostko kostko commented Feb 15, 2023

No description provided.

@matevz
Copy link
Member

matevz commented Feb 16, 2023

This is probably related to #8 ?

I'm puzzled how to test this. So I tried:

$ oasis accounts transfer 1 test:bob --offline --gas-limit 1 --nonce 1 --wallet test:alice --network testnet --no-paratime
You are about to sign the following transaction:
Method: staking.Transfer
Body:
  To:     oasis1qrydpazemvuwtnp3efm7vmfvg3tde044qg6cxwzx
  Amount: 1.0 TEST
Nonce:  1
Fee:
  Amount: 0.0 TEST
  Gas limit: 1
  (gas price: 0.0 TEST per gas unit)

Account:  test:alice
Network:  testnet
Paratime: none (consensus layer)
? Sign this transaction? Yes
(In case you are using a hardware-based signer you may need to confirm on device.)
{
  "untrusted_raw_value": "pGNmZWWiY2dhcwFmYW1vdW50QGRib2R5omJ0b1UAyND0Wds45cwxynfmbSxEVty+tQJmYW1vdW50RDuaygBlbm9uY2UBZm1ldGhvZHBzdGFraW5nLlRyYW5zZmVy",
  "signature": {
    "public_key": "NcPzNW3YU2T+ugNUtUWtoQnRvbOL9dYSaBfbjHLP1pE=",
    "signature": "k51gq/PyDHCjm2vEMMpePnnTOaFUddRadfrP8UmgzMz1G0JDr+1lRH9lsg04+2krpmMYs7TOgX55+RVJ5a+LDw=="
  }
}

and copied the content into testtx.json.

Then, tx show gives me INVALID SIGNATURE error:

$ oasis tx show testtx.json 
Hash: bd312d0faf8b7399b8b92b07cb6b40b418995875551341fa8540503322c5b70a
Signer: NcPzNW3YU2T+ugNUtUWtoQnRvbOL9dYSaBfbjHLP1pE=
        (signature: k51gq/PyDHCjm2vEMMpePnnTOaFUddRadfrP8UmgzMz1G0JDr+1lRH9lsg04+2krpmMYs7TOgX55+RVJ5a+LDw==)
        [INVALID SIGNATURE]
Content:
  Method: staking.Transfer
  Body:
    To:     oasis1qrydpazemvuwtnp3efm7vmfvg3tde044qg6cxwzx
    Amount: 1.0 TEST
  Nonce:  1
  Fee:
    Amount: 0.0 TEST
    Gas limit: 1
    (gas price: 0.0 TEST per gas unit)

Showing runtime tx also seems wrong:

$ oasis accounts transfer 1 test:bob --offline --gas-limit 1 --nonce 1 --wallet test:alice --network testnet --paratime sapphire
You are about to sign the following transaction:
{
  "v": 1,
  "call": {
    "method": "accounts.Transfer",
    "body": "omJ0b1UAyND0Wds45cwxynfmbSxEVty+tQJmYW1vdW50gkgN4Lazp2QAAEA="
  },
  "ai": {
    "si": [
      {
        "address_spec": {
          "signature": {
            "ed25519": "NcPzNW3YU2T+ugNUtUWtoQnRvbOL9dYSaBfbjHLP1pE="
          }
        },
        "nonce": 1
      }
    ],
    "fee": {
      "amount": {
        "Amount": "0",
        "Denomination": ""
      },
      "gas": 1
    }
  }
}

Account:  test:alice
Network:  testnet
Paratime: sapphire
? Sign this transaction? Yes
(In case you are using a hardware-based signer you may need to confirm on device.)
{
  "Body": "o2F2AWJhaaJic2mBomVub25jZQFsYWRkcmVzc19zcGVjoWlzaWduYXR1cmWhZ2VkMjU1MTlYIDXD8zVt2FNk/roDVLVFraEJ0b2zi/XWEmgX24xyz9aRY2ZlZaJjZ2FzAWZhbW91bnSCQEBkY2FsbKJkYm9keaJidG9VAMjQ9FnbOOXMMcp35m0sRFbcvrUCZmFtb3VudIJIDeC2s6dkAABAZm1ldGhvZHFhY2NvdW50cy5UcmFuc2Zlcg==",
  "AuthProofs": [
    {
      "signature": "mxOARaaRkHrHiUObGO2KbUXZhmbIS2h3hsRM+3Y/DzvMtbVdwcZm2EZ7oP7dR5XHKkrQnjMwZnkT0mdZUsLIBw=="
    }
  ]
}

$ oasis tx show testtx2.json 
Hash: ff6578af32b265a25fad72546667b078df7a4ca68a519c2ee6d2b3fe4df7d4d9
Signer: AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=
        (signature: AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA==)
        [INVALID SIGNATURE]
Content:
  Method: 
  Body:
    <unknown method body: >
  Nonce:  0
  Fee:   none

@kostko
Copy link
Member Author

kostko commented Feb 16, 2023

No, #8 is improving the "show tx" for ParaTime transactions. This just enables showing transactions in general, the actual pretty output can be improved in the future.

But yeah I see there are some problems with format autodetection, will take a look ;-)

@kostko
Copy link
Member Author

kostko commented Feb 16, 2023

@matevz The transaction decoding should work correctly now and there should no longer be signature verification errors as long as you use the right --network during tx show. Note that this does not improve how signed or unsigned runtime transactions are actually pretty-printed (e.g. currently it is just an ugly JSON dump, so not much added value there) as that is something that should be solved in #8 (and this new tx show would then automatically use that improved output as well).

},
}
)

func tryDecodeTx(rawTx []byte) (any, error) {
Copy link
Member

Choose a reason for hiding this comment

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

Should this go into oasis-sdk? E.g. to simplify the runtime test vectors and for debugging future client-sdk/go apps?

Copy link
Member Author

Choose a reason for hiding this comment

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

I would rather not bloat the SDK until this is required somewhere else as well.

cmd/tx.go Outdated Show resolved Hide resolved
@kostko kostko force-pushed the kostko/feature/tx-show branch 3 times, most recently from 90ea20a to e624f97 Compare February 17, 2023 12:37
Copy link
Member

@matevz matevz left a comment

Choose a reason for hiding this comment

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

Thanks, good job!

cmd/common/transaction.go Outdated Show resolved Hide resolved
@kostko kostko merged commit 8bea824 into master Feb 17, 2023
@kostko kostko deleted the kostko/feature/tx-show branch February 17, 2023 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants