improve client tx result handling #2276

tzemanovic · 2023-12-13T17:51:47Z

Describe your changes

closes refactor TxResult event #20

The TxResponse type is updated to contain a typed inner_tx: TxResult for inner txs and some of its other fields are now typed. This is to make it possible to check a tx result in the client before taking further actions that depend on the result.

Stdout from a transaction now looks like this:

Transaction added to mempool.
Wrapper transaction hash: 137DE5305AC348B216D956AC538E6D23E233B20847C7B7EC266F48F2C3BA2CE1
Inner transaction hash: 89A8DBE501CC141508FBD54B9ED747AE3800A3BA299045323A7139988BBA3472
Wrapper transaction accepted at height 202. Used 21 gas.
Waiting for inner transaction result...
Transaction was successfully applied at height 203. Used 7890 gas.

A failed tx prints more info, e.g.:

Transaction added to mempool.
Wrapper transaction hash: 6E9083C5786E144B2BB190467FDC392129E8BC8D59AA1B394A4606F4E1EC5E02
Inner transaction hash: 619B2531D37A5F7F81787B4D5401158579647DDBEFB18BD9C5D4C3414B429777
Wrapper transaction accepted at height 172. Used 20 gas.
Waiting for inner transaction result...
Transaction was rejected by VPs: [
  "tnam1q9rhgyv3ydq0zu3whnftvllqnvhvhm270qxay5tn"
].
Changed keys: [
  "#tnam1pyqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqej6juv/#tnam1q9mjvqd45u7w54kee2aquugtv7vn7h3xrcrau7xy/balance/#tnam1q9rhgyv3ydq0zu3whnftvllqnvhvhm270qxay5tn",
  "#tnam1pyqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqej6juv/#tnam1q9mjvqd45u7w54kee2aquugtv7vn7h3xrcrau7xy/balance/#tnam1qxfj3sf6a0meahdu9t6znp05g8zx4dkjtgyn9gfu"
]

Indicate on which release or other PRs this topic is based on

#2218 - diff range for review https://github.com/anoma/namada/pull/2276/files/dd28da14cefd6806ebfe71eb6a8ba8c894d975cd..f167f2b5b8396acf148fcc9cf992daf89f3b48a7

Checklist before merging to `draft`

I have added a changelog
Git history is in acceptable state

apps/src/lib/client/tx.rs

sug0

couple of suggestions

apps/src/lib/client/tx.rs

core/src/types/transaction/mod.rs

sdk/src/tx.rs

tests/src/e2e/ibc_tests.rs

tests/src/integration/masp.rs

Co-authored-by: Tiago Carvalho <sugoiuguu@tfwno.gf>

* origin/main: ci: update mold ci: split integration and unit

* tomas/nicer-client-tx-result: Merge remote-tracking branch 'origin/main' refactor and use ProcessTxResponse::is_applied_and_valid core: add a note on transaction ResultCode Update sdk/src/tx.rs tests: add consts for ledger startup exp strings tests: replace expected tx failure string with a const tests: replace expected tx strings with consts changelog: add #2276 Rename ErrorCodes to ResultCode and make TxReponse.height typed sdk: use typed `ErrorCodes` for `TxResponse` move tx ErrorCodes type to core sdk: remove `initialized_accounts` from `TxResponse` (it's in inner_tx) tests: update expected tx result strings refactor tx response and result sdk/tx: remove unused `ProcessTxResponse` case

* origin/tomas/nicer-client-tx-result: Merge remote-tracking branch 'origin/main' refactor and use ProcessTxResponse::is_applied_and_valid core: add a note on transaction ResultCode Update sdk/src/tx.rs tests: add consts for ledger startup exp strings tests: replace expected tx failure string with a const tests: replace expected tx strings with consts changelog: add #2276 Rename ErrorCodes to ResultCode and make TxReponse.height typed sdk: use typed `ErrorCodes` for `TxResponse` move tx ErrorCodes type to core sdk: remove `initialized_accounts` from `TxResponse` (it's in inner_tx) tests: update expected tx result strings refactor tx response and result sdk/tx: remove unused `ProcessTxResponse` case

tzemanovic force-pushed the tomas/nicer-client-tx-result branch 3 times, most recently from 4cc7e5c to ea0a59b Compare December 14, 2023 10:45

tzemanovic commented Dec 14, 2023

View reviewed changes

apps/src/lib/client/tx.rs Outdated Show resolved Hide resolved

tzemanovic force-pushed the tomas/nicer-client-tx-result branch from ea0a59b to 306a634 Compare December 14, 2023 10:53

tzemanovic added a commit that referenced this pull request Dec 14, 2023

changelog: add #2276

627e66c

tzemanovic marked this pull request as ready for review December 14, 2023 10:58

tzemanovic requested a review from sug0 December 14, 2023 11:00

tzemanovic mentioned this pull request Dec 14, 2023

fix tx result handling #2279

Merged

2 tasks

sug0 requested changes Dec 14, 2023

View reviewed changes

brentstone and others added 11 commits December 15, 2023 15:12

WIP

100f882

fix bug

ae8963f

need unsafe-dont-encrypt arg for testing

4cc20c2

test/e2e/ledger: finish consensus key change test

ae77eff

test/e2e: make Who copy-able

fdf9008

github/workflows: add e2e test

bbde7ea

client/tx: tidy up change-consensus-key

6d94864

changelog: add #2218

de2e176

benches/tx: fix change_consensus_key by adding the extra req sig

dd28da1

sdk/tx: remove unused ProcessTxResponse case

936a95d

refactor tx response and result

e18787c

brentstone force-pushed the tomas/nicer-client-tx-result branch from 627e66c to 54bba83 Compare December 15, 2023 20:18

brentstone pushed a commit that referenced this pull request Dec 15, 2023

changelog: add #2276

54bba83

brentstone mentioned this pull request Dec 15, 2023

Draft #2252

Closed

tzemanovic added 6 commits December 19, 2023 16:42

tests: update expected tx result strings

79317bb

sdk: remove initialized_accounts from TxResponse (it's in inner_tx)

238bd81

move tx ErrorCodes type to core

30cb489

sdk: use typed ErrorCodes for TxResponse

e19e65e

Rename ErrorCodes to ResultCode and make TxReponse.height typed

eb909db

changelog: add #2276

f167f2b

tzemanovic force-pushed the tomas/nicer-client-tx-result branch from 4484648 to f167f2b Compare December 19, 2023 16:42

tzemanovic and others added 7 commits December 19, 2023 17:16

tests: replace expected tx strings with consts

68db8f6

tests: replace expected tx failure string with a const

fe0d403

tests: add consts for ledger startup exp strings

d09bc55

Update sdk/src/tx.rs

211bfc5

Co-authored-by: Tiago Carvalho <sugoiuguu@tfwno.gf>

core: add a note on transaction ResultCode

c0fc84b

refactor and use ProcessTxResponse::is_applied_and_valid

42f05af

Merge remote-tracking branch 'origin/main'

ab32945

* origin/main: ci: update mold ci: split integration and unit

brentstone added the merged to draft label Dec 19, 2023

brentstone merged commit 5a9da48 into main Dec 29, 2023
14 of 15 checks passed

brentstone deleted the tomas/nicer-client-tx-result branch December 29, 2023 19:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

improve client tx result handling #2276

improve client tx result handling #2276

tzemanovic commented Dec 13, 2023 •

edited

Loading

sug0 left a comment

improve client tx result handling #2276

improve client tx result handling #2276

Conversation

tzemanovic commented Dec 13, 2023 • edited Loading

Describe your changes

Indicate on which release or other PRs this topic is based on

Checklist before merging to draft

sug0 left a comment

Choose a reason for hiding this comment

tzemanovic commented Dec 13, 2023 •

edited

Loading

Checklist before merging to `draft`