Page MenuHomePhabricator
Differential D11711

[Cashtab] [Chronik] [Tx Gen] Broadcast Tx
AbandonedPublic
Actions

Authored by emack on Jul 5 2022, 05:48.

Details

Reviewers
bytesofman
Group Reviewers
Restricted Project
Summary

As per T2503, this is part of a series of diffs to refactor the transaction generation process within useBCH.

This diff is a standalone utility function which signs the input utxos and broadcasts the raw tx hex via the chronik client.

This function returns a mainnet or testnet explorer link.

Test Plan

jest mocks for chronik api calls are WIP

Diff Detail

Repository
rABC Bitcoin ABC
Branch
chronikBroadcast
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 19588
Build 38895: Build Diffcashtab-tests
Build 38894: arc lint + arc unit

Event Timeline

emack created this revision.Jul 5 2022, 05:48
Herald added a reviewer: Restricted Project. · View Herald TranscriptJul 5 2022, 05:48
emack requested review of this revision.Jul 5 2022, 05:48
Harbormaster completed remote builds in B19554: Diff 34244.Jul 5 2022, 05:52
bytesofman requested changes to this revision.Jul 5 2022, 18:02
bytesofman added inline comments.
web/cashtab/src/utils/cashMethods.js
20 ↗(On Diff #34244)

What does txidStr look like in this chronik response?

Include a mock in code comments.

This revision now requires changes to proceed.Jul 5 2022, 18:02
emack updated this revision to Diff 34269.Jul 6 2022, 04:55
emack marked an inline comment as done.

Good call on checking this. The broadcast response from Chronik is actually an object with a txid param as opposed to bch-js' array of txids. Have adjusted txid parsing logic accordingly.

For reference:

  • chronik.broadcastTx() response

{"txid":"0075130c9ecb342b5162bb1a8a870e69c935ea0c9b2353a967cda404401acf19"}

  • BCH.RawTransactions.sendRawTransaction() response

["757f3aeb387fbc653cc7452729060cc42bf083916674f81891b7fc0af1a0c949"]

Harbormaster completed remote builds in B19573: Diff 34269.Jul 6 2022, 04:58
emack edited the summary of this revision. (Show Details)Jul 6 2022, 05:23
bytesofman requested changes to this revision.Jul 6 2022, 23:23
bytesofman added inline comments.
web/cashtab/src/utils/cashMethods.js
11 ↗(On Diff #34269)

We only want to have one ChronikClient in the app. Currently that exists in useWallet.js

It will need to be an input to the function similar to how BCH is right now.

This revision now requires changes to proceed.Jul 6 2022, 23:23
emack updated this revision to Diff 34293.Jul 7 2022, 12:09
emack marked an inline comment as done.

chronik instance passed in as an input param

Harbormaster completed remote builds in B19588: Diff 34293.Jul 7 2022, 12:12
bytesofman requested changes to this revision.Jul 7 2022, 17:56
bytesofman added inline comments.
web/cashtab/src/utils/cashMethods.js
11

Seeing the function in this style, I think it's best to

  1. Forget about the testnet vs mainnet catches. Cashtab does not support testnet currently and at this point it's unlikely we will work in that support. We would need to scan the whole app and figure out what would actually work with testnet and what is an artifact from a couple of years back.
  1. I don't think we need a function for this. Instead, we should incorporate try...catch blocks that call await chronik.broadcastTx(rawTxHex) in each tx creation function. There could be some repetition in creating the link tx string -- perhaps we will make a wrapper function for that, something like getLinkFromTxid(), but even that seems to offer marginal value once we drop the testnet brackets.

So -- recommended action here is to abandon this diff.

This revision now requires changes to proceed.Jul 7 2022, 17:56
emack abandoned this revision.Jul 8 2022, 00:03

To be incorporated directly into each tx creation function.

Revision Contents
Changeset List

PathSize
web/
cashtab/
src/
utils/
23 lines

Diff 34293

View Options

web/cashtab/src/utils/cashMethods.js

Loading...