Page MenuHomePhabricator
Differential D18833

[ecash-agora, ecash-wallet, ecash-lib] Add a new take method to the AgoraOffer class that automatically builds and broadcasts an acceptTx
ClosedPublic
Actions

Authored by bytesofman on Sat, Oct 25, 22:26.

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Commits
rABCda5d0a7ac2f1: [ecash-agora, ecash-wallet, ecash-lib] Add a new take method to the AgoraOffer…
Summary

ecash-agora is a powerful library that will allow transaction building to accept agora offers. However, since there is no wallet logic in ecash-agora, there is some trickiness with passing params and determining utxos.

It makes sense for ecash-wallet to "just handle" agora stuff. However, it does not really make sense to import the whole ecash-agora lib into ecash-wallet to solve this problem.

Follow the existing API of the acceptTx method (which builds a tx given user-determined wallet inputs), and add a new take method that does this automatically by passing a Wallet.

Test Plan

npm test, CI for integration tests

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

bytesofman created this revision.Sat, Oct 25, 22:26
Herald added a reviewer: Restricted Project. · View Herald TranscriptSat, Oct 25, 22:26
Harbormaster completed remote builds in B34860: Diff 56294.Sat, Oct 25, 22:30
teamcity added a comment.Sat, Oct 25, 22:30
Build Bitcoin ABC Diffs / Diff Testing (cashtab-tests) failed.

Tail of the build log:

Run `npm audit` for details.

> chronik-client@3.4.0 build
> tsc

/work/modules/ecash-lib /work/abc-ci-builds/cashtab-tests

added 271 packages, and audited 275 packages in 1s

37 packages are looking for funding
  run `npm fund` for details

7 vulnerabilities (1 low, 5 moderate, 1 high)

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

> ecash-lib@4.4.0 build
> tsc && tsc -p ./tsconfig.build.json && cp -r ./src/ffi ./dist

/work/modules/ecash-wallet /work/abc-ci-builds/cashtab-tests

added 221 packages, and audited 224 packages in 2s

57 packages are looking for funding
  run `npm fund` for details

3 low severity vulnerabilities

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

> ecash-wallet@2.0.2 build
> tsc -p ./tsconfig.build.json

/work/modules/ecash-agora /work/abc-ci-builds/cashtab-tests

added 273 packages, and audited 277 packages in 1s

37 packages are looking for funding
  run `npm fund` for details

7 vulnerabilities (1 low, 5 moderate, 1 high)

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

> ecash-agora@2.4.0 build
> tsc && tsc -p ./tsconfig.build.json

tests/partial.alp.test.ts(640,17): error TS2353: Object literal may only specify known properties, and 'takerInput' does not exist in type '{ chronik: ChronikClient; offer: AgoraOffer; takerSk: Uint8Array; acceptedAtoms: bigint; allowUnspendable?: boolean | undefined; }'.
tests/partial.alp.test.ts(704,17): error TS2353: Object literal may only specify known properties, and 'takerInput' does not exist in type '{ chronik: ChronikClient; offer: AgoraOffer; takerSk: Uint8Array; acceptedAtoms: bigint; allowUnspendable?: boolean | undefined; }'.
Build cashtab-tests failed with exit code 2
teamcity added a comment.Sat, Oct 25, 22:35
Build Bitcoin ABC Diffs / Diff Testing (ecash-agora-integration-tests) failed.

Tail of the build log:

[557/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/trafficgraphwidget.cpp.o
[558/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/addressbookpage.cpp.o
[559/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/coincontroltreewidget.cpp.o
[560/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/utilitydialog.cpp.o
[561/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/createwalletdialog.cpp.o
[562/596] Linking CXX executable src/bench/bitcoin-bench
[563/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/splashscreen.cpp.o
[564/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/editaddressdialog.cpp.o
[565/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/qrc_bitcoin_locale.cpp.o
[566/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/askpassphrasedialog.cpp.o
[567/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/qrimagewidget.cpp.o
[568/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/openuridialog.cpp.o
[569/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/addresstablemodel.cpp.o
[570/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/overviewpage.cpp.o
[571/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/transactionfilterproxy.cpp.o
[572/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/receiverequestdialog.cpp.o
[573/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/transactiondescdialog.cpp.o
[574/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/recentrequeststablemodel.cpp.o
[575/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/sendcoinsentry.cpp.o
[576/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/coincontroldialog.cpp.o
[577/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/rpcconsole.cpp.o
[578/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/receivecoinsdialog.cpp.o
[579/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/transactionrecord.cpp.o
[580/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/paymentserver.cpp.o
[581/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/walletmodeltransaction.cpp.o
[582/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/signverifymessagedialog.cpp.o
[583/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/transactionview.cpp.o
[584/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/walletframe.cpp.o
[585/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/paymentrequestplus.cpp.o
[586/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/transactiontablemodel.cpp.o
[587/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/transactiondesc.cpp.o
[588/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/walletcontroller.cpp.o
[589/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/sendcoinsdialog.cpp.o
[590/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/walletview.cpp.o
[591/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/walletmodel.cpp.o
[592/596] Linking CXX static library src/qt/libbitcoin-qt-base.a
[593/596] Automatic MOC for target bitcoin-qt
[594/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt.dir/bitcoin-qt_autogen/mocs_compilation.cpp.o
[595/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt.dir/main.cpp.o
[596/596] Linking CXX executable src/qt/bitcoin-qt
/work/modules/ecash-agora /work/abc-ci-builds/ecash-agora-integration-tests

added 273 packages, and audited 277 packages in 1s

37 packages are looking for funding
  run `npm fund` for details

7 vulnerabilities (1 low, 5 moderate, 1 high)

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

> ecash-agora@2.4.0 build
> tsc && tsc -p ./tsconfig.build.json

tests/partial.alp.test.ts(640,17): error TS2353: Object literal may only specify known properties, and 'takerInput' does not exist in type '{ chronik: ChronikClient; offer: AgoraOffer; takerSk: Uint8Array; acceptedAtoms: bigint; allowUnspendable?: boolean | undefined; }'.
tests/partial.alp.test.ts(704,17): error TS2353: Object literal may only specify known properties, and 'takerInput' does not exist in type '{ chronik: ChronikClient; offer: AgoraOffer; takerSk: Uint8Array; acceptedAtoms: bigint; allowUnspendable?: boolean | undefined; }'.
Build ecash-agora-integration-tests failed with exit code 2
bytesofman updated this revision to Diff 56295.Sat, Oct 25, 22:36

rebase for chronik-client fix

Harbormaster completed remote builds in B34861: Diff 56295.Sat, Oct 25, 22:40
teamcity added a comment.Sat, Oct 25, 22:40
Build Bitcoin ABC Diffs / Diff Testing (cashtab-tests) failed.

Tail of the build log:

Run `npm audit` for details.

> chronik-client@3.4.0 build
> tsc

/work/modules/ecash-lib /work/abc-ci-builds/cashtab-tests

added 271 packages, and audited 275 packages in 1s

37 packages are looking for funding
  run `npm fund` for details

7 vulnerabilities (1 low, 5 moderate, 1 high)

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

> ecash-lib@4.4.0 build
> tsc && tsc -p ./tsconfig.build.json && cp -r ./src/ffi ./dist

/work/modules/ecash-wallet /work/abc-ci-builds/cashtab-tests

added 221 packages, and audited 224 packages in 2s

57 packages are looking for funding
  run `npm fund` for details

3 low severity vulnerabilities

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

> ecash-wallet@2.0.2 build
> tsc -p ./tsconfig.build.json

/work/modules/ecash-agora /work/abc-ci-builds/cashtab-tests

added 273 packages, and audited 277 packages in 1s

37 packages are looking for funding
  run `npm fund` for details

7 vulnerabilities (1 low, 5 moderate, 1 high)

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

> ecash-agora@2.4.0 build
> tsc && tsc -p ./tsconfig.build.json

tests/partial.alp.test.ts(640,17): error TS2353: Object literal may only specify known properties, and 'takerInput' does not exist in type '{ chronik: ChronikClient; offer: AgoraOffer; takerSk: Uint8Array; acceptedAtoms: bigint; allowUnspendable?: boolean | undefined; }'.
tests/partial.alp.test.ts(704,17): error TS2353: Object literal may only specify known properties, and 'takerInput' does not exist in type '{ chronik: ChronikClient; offer: AgoraOffer; takerSk: Uint8Array; acceptedAtoms: bigint; allowUnspendable?: boolean | undefined; }'.
Build cashtab-tests failed with exit code 2
bytesofman updated this revision to Diff 56296.Sat, Oct 25, 22:42

lint fix

teamcity added a comment.Sat, Oct 25, 22:43
Build Bitcoin ABC Diffs / Diff Testing (ecash-agora-integration-tests) failed.

Tail of the build log:

[557/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/utilitydialog.cpp.o
[558/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/trafficgraphwidget.cpp.o
[559/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/addressbookpage.cpp.o
[560/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/splashscreen.cpp.o
[561/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/qrc_bitcoin_locale.cpp.o
[562/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/coincontroltreewidget.cpp.o
[563/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/createwalletdialog.cpp.o
[564/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/rpcconsole.cpp.o
[565/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/editaddressdialog.cpp.o
[566/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/openuridialog.cpp.o
[567/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/addresstablemodel.cpp.o
[568/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/qrimagewidget.cpp.o
[569/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/coincontroldialog.cpp.o
[570/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/overviewpage.cpp.o
[571/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/askpassphrasedialog.cpp.o
[572/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/transactionfilterproxy.cpp.o
[573/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/transactiondescdialog.cpp.o
[574/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/paymentserver.cpp.o
[575/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/receivecoinsdialog.cpp.o
[576/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/receiverequestdialog.cpp.o
[577/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/signverifymessagedialog.cpp.o
[578/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/transactiondesc.cpp.o
[579/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/transactionrecord.cpp.o
[580/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/sendcoinsentry.cpp.o
[581/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/transactiontablemodel.cpp.o
[582/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/recentrequeststablemodel.cpp.o
[583/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/transactionview.cpp.o
[584/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/sendcoinsdialog.cpp.o
[585/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/walletframe.cpp.o
[586/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/walletmodeltransaction.cpp.o
[587/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/walletcontroller.cpp.o
[588/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/paymentrequestplus.cpp.o
[589/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/walletview.cpp.o
[590/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/walletmodel.cpp.o
[591/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/qrc_bitcoin.cpp.o
[592/596] Linking CXX static library src/qt/libbitcoin-qt-base.a
[593/596] Automatic MOC for target bitcoin-qt
[594/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt.dir/bitcoin-qt_autogen/mocs_compilation.cpp.o
[595/596] Building CXX object src/qt/CMakeFiles/bitcoin-qt.dir/main.cpp.o
[596/596] Linking CXX executable src/qt/bitcoin-qt
/work/modules/ecash-agora /work/abc-ci-builds/ecash-agora-integration-tests

added 273 packages, and audited 277 packages in 1s

37 packages are looking for funding
  run `npm fund` for details

7 vulnerabilities (1 low, 5 moderate, 1 high)

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

> ecash-agora@2.4.0 build
> tsc && tsc -p ./tsconfig.build.json

tests/partial.alp.test.ts(640,17): error TS2353: Object literal may only specify known properties, and 'takerInput' does not exist in type '{ chronik: ChronikClient; offer: AgoraOffer; takerSk: Uint8Array; acceptedAtoms: bigint; allowUnspendable?: boolean | undefined; }'.
tests/partial.alp.test.ts(704,17): error TS2353: Object literal may only specify known properties, and 'takerInput' does not exist in type '{ chronik: ChronikClient; offer: AgoraOffer; takerSk: Uint8Array; acceptedAtoms: bigint; allowUnspendable?: boolean | undefined; }'.
Build ecash-agora-integration-tests failed with exit code 2
Harbormaster completed remote builds in B34862: Diff 56296.Sat, Oct 25, 22:52
bytesofman published this revision for review.Sat, Oct 25, 23:01
Fabien requested changes to this revision.Mon, Oct 27, 14:23
Fabien added a subscriber: Fabien.
Fabien added inline comments.
modules/ecash-agora/src/agora.ts
194 ↗(On Diff #56296)

The name of the function is bad, this might come to a surprise to a developer that is wallet is drained by an "accept" function. Can you find a more explicit name?

196 ↗(On Diff #56296)
201 ↗(On Diff #56296)
212 ↗(On Diff #56296)
217 ↗(On Diff #56296)
This revision now requires changes to proceed.Mon, Oct 27, 14:23
bytesofman marked 5 inline comments as done.Mon, Oct 27, 16:56
bytesofman added inline comments.
modules/ecash-agora/src/agora.ts
194 ↗(On Diff #56296)

k going with take()

bytesofman updated this revision to Diff 56315.Mon, Oct 27, 17:09
bytesofman marked an inline comment as done.

patch comment typos, rename method

Harbormaster completed remote builds in B34876: Diff 56315.Mon, Oct 27, 17:26
teamcity added a comment.Mon, Oct 27, 17:26
Build Bitcoin ABC Diffs / Diff Testing (ecash-agora-integration-tests) failed.

Failed tests logs:

====== AgoraPartial SLP: AgoraPartial SLP 1000 for 1sat/token, dust accept, must allowUnspendable.AgoraPartial SLP AgoraPartial SLP 1000 for 1sat/token, dust accept, must allowUnspendable ======
Error: Insufficient input sats (2333): Can only pay for 695 fees, but 1186 required
    at TxBuilder.sign (/work/modules/ecash-lib/src/txBuilder.ts:204:23)
    at AgoraOffer.take (src/agora.ts:294:34)
    at takeSlpOffer (tests/partial-helper-slp.ts:144:48)
    at process.processTicksAndRejections (node:internal/process/task_queues:105:5)
    at async Context.<anonymous> (tests/partial.slp.test.ts:498:32)

Each failure log is accessible here:
AgoraPartial SLP: AgoraPartial SLP 1000 for 1sat/token, dust accept, must allowUnspendable.AgoraPartial SLP AgoraPartial SLP 1000 for 1sat/token, dust accept, must allowUnspendable

bytesofman mentioned this in D18840: [ecash-wallet] Export the BuiltAction class so it is available for ecash-agora take method.Mon, Oct 27, 17:31
bytesofman planned changes to this revision.Mon, Oct 27, 17:53

will get D18840 first, check out the CI

modules/ecash-wallet/README.md
117 ↗(On Diff #56315)

NB will change this to take before landing

Fabien added inline comments.Mon, Oct 27, 19:32
modules/ecash-wallet/README.md
117 ↗(On Diff #56315)

you'd better change it then rebase so CI is green

bytesofman mentioned this in rABCd7396424d047: [ecash-wallet] Export the BuiltAction class so it is available for ecash-agora….Mon, Oct 27, 19:39
bytesofman updated this revision to Diff 56321.Mon, Oct 27, 19:44

rebase

bytesofman retitled this revision from [ecash-agora, ecash-wallet, ecash-lib] Add a new accept method to the AgoraOffer class that automatically builds and broadcasts an accept tx to [ecash-agora, ecash-wallet, ecash-lib] Add a new take method to the AgoraOffer class that automatically builds and broadcasts an acceptTx.Mon, Oct 27, 19:45
bytesofman edited the summary of this revision. (Show Details)
Harbormaster completed remote builds in B34880: Diff 56321.Mon, Oct 27, 20:18
Fabien accepted this revision.Mon, Oct 27, 20:32
This revision is now accepted and ready to land.Mon, Oct 27, 20:32
Closed by commit rABCda5d0a7ac2f1: [ecash-agora, ecash-wallet, ecash-lib] Add a new take method to the AgoraOffer… (authored by bytesofman). · Explain WhyMon, Oct 27, 20:38
This revision was automatically updated to reflect the committed changes.
bytesofman added a commit: rABCda5d0a7ac2f1: [ecash-agora, ecash-wallet, ecash-lib] Add a new take method to the AgoraOffer….
bytesofman mentioned this in D18844: [ecash-lib] Patch ecash-agora CI deployments.Tue, Oct 28, 12:19
bytesofman mentioned this in rABCcfb071603a70: [ecash-lib] Patch ecash-agora CI deployments.Tue, Oct 28, 12:59

Revision Contents
Changeset List

PathSize
modules/
ecash-agora/
4 lines
6 lines
2 lines
src/
125 lines
tests/
18 lines
17 lines
41 lines
22 lines
41 lines
23 lines
ecash-lib/
src/
test/
36 lines

Diff 56322

View Options

modules/ecash-agora/README.md

Loading...
View Options

modules/ecash-agora/package-lock.json

Loading...
View Options

modules/ecash-agora/package.json

Loading...
View Options

modules/ecash-agora/src/agora.ts

Loading...
View Options

modules/ecash-agora/tests/partial-helper-alp.ts

Loading...
View Options

modules/ecash-agora/tests/partial-helper-slp.ts

Loading...
View Options

modules/ecash-agora/tests/partial.alp.bigsats.test.ts

Loading...
View Options

modules/ecash-agora/tests/partial.alp.test.ts

Loading...
View Options

modules/ecash-agora/tests/partial.slp.bigsats.test.ts

Loading...
View Options

modules/ecash-agora/tests/partial.slp.test.ts

Loading...
View Options

modules/ecash-lib/src/test/testRunner.ts

Loading...