Page MenuHomePhabricator

[chronik-client] Support tx and block-txs endpoints in ChronikClientNode
ClosedPublic

Authored by bytesofman on Jan 25 2024, 05:05.

Details

Summary

Add support for the chronik.tx(txid) and chronik.blockTxs(hashOrHeight, pageNumber, pageSize) endpoints in the in-node version of chronik-client

Convert value key from string to number. Convert timestamp and timeFirstSeen keys from string to number.

Test Plan

teamcity CI tests in this diff, or see local testing approach from test plan at D14915

Diff Detail

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

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

Failed tests logs:

====== /chronik-info: "before each" hook for "gives us the chronik info and throws expected error on bad server connection"./chronik-info "before each" hook for "gives us the chronik info and throws expected error on bad server connection" ======
Error: Timeout of 2000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/work/modules/chronik-client/test/integration/chronik_info.ts)
    at listOnTimeout (node:internal/timers:559:17)
    at processTimers (node:internal/timers:502:7)

Each failure log is accessible here:
/chronik-info: "before each" hook for "gives us the chronik info and throws expected error on bad server connection"./chronik-info "before each" hook for "gives us the chronik info and throws expected error on bad server connection"

add support for blockTxs endpoint and some integration tests

rename test file for combined integration tests, add tests for blockTxs and tx

more tests, code org, changelog update

test unconfirmed txs and confirmed txs

doxygen comment formatting fix, change var name to plural for array of txids

bytesofman retitled this revision from [chronik-client] Support tx endpoint in node to [chronik-client] Support tx and block-txs endpoints in ChronikClientNode.
bytesofman edited the summary of this revision. (Show Details)
bytesofman edited the test plan for this revision. (Show Details)

Failed tests logs:

====== Gets available non-script endpoint info: "before each" hook for "New regtest chain".Gets available non-script endpoint info "before each" hook for "New regtest chain" ======
Error: Timeout of 2000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/work/modules/chronik-client/test/integration/non_script_endpoints.ts)
    at listOnTimeout (node:internal/timers:559:17)
    at processTimers (node:internal/timers:502:7)

Each failure log is accessible here:
Gets available non-script endpoint info: "before each" hook for "New regtest chain".Gets available non-script endpoint info "before each" hook for "New regtest chain"

Fabien requested changes to this revision.Jan 30 2024, 08:29
Fabien added a subscriber: Fabien.

The test is failing, back to your queue

This revision now requires changes to proceed.Jan 30 2024, 08:29
Fabien requested changes to this revision.Jan 30 2024, 15:41

This deserves to be a separate test and setup, I don't see the point of having it all in one as the steps are totally not correlated

modules/chronik-client/src/ChronikClientNode.ts
56 โ†—(On Diff #44722)

Note: should be between 1 and 200, but certainly better to let the server handle the error here.

Why not use a default value here ? Server default is 25. I don't understand why both args are not handled the same way, either both optional (the server will use defaults) or both with a sane default.

modules/chronik-client/test/integration/non_script_endpoints.ts
11 โ†—(On Diff #44722)
This revision now requires changes to proceed.Jan 30 2024, 15:41
bytesofman added inline comments.
modules/chronik-client/src/ChronikClientNode.ts
56 โ†—(On Diff #44722)

yeah it makes sense to have them both set to defaults if unspecified.

bytesofman marked an inline comment as done.

default params, get instead of gets

Fabien requested changes to this revision.Jan 30 2024, 19:53

This deserves to be a separate test and setup, I don't see the point of having it all in one as the steps are totally not correlated

modules/chronik-client/src/ChronikClientNode.ts
266โ€“269 โ†—(On Diff #44744)
This revision now requires changes to proceed.Jan 30 2024, 19:53

split out setup scripts and tests, fix comment on txid

Fabien requested changes to this revision.Jan 30 2024, 21:47
Fabien added inline comments.
test/functional/setup_scripts/chronik-client_blocktxs_and_tx.py
42 โ†—(On Diff #44749)

remove

48 โ†—(On Diff #44749)

The generate() method returns a list of the generated block hashes. Since you don't store the tip them you don't need to access the last array element

51 โ†—(On Diff #44749)

You can park the block as another step and check the txs are still available in the mempool

This revision now requires changes to proceed.Jan 30 2024, 21:47
test/functional/setup_scripts/chronik-client_blocktxs_and_tx.py
50 โ†—(On Diff #44749)

yield False for the last step. This makes the setup to raise an error if this is unexpected and ensure both TS and python are in sync

Failed tests logs:

====== /chronik-info: "before each" hook for "gives us the chronik info and throws expected error on bad server connection"./chronik-info "before each" hook for "gives us the chronik info and throws expected error on bad server connection" ======
Error: Timeout of 2000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/work/modules/chronik-client/test/integration/chronik_info.ts)
    at listOnTimeout (node:internal/timers:559:17)
    at processTimers (node:internal/timers:502:7)

Each failure log is accessible here:
/chronik-info: "before each" hook for "gives us the chronik info and throws expected error on bad server connection"./chronik-info "before each" hook for "gives us the chronik info and throws expected error on bad server connection"

Failed tests logs:

====== /chronik-info: "before each" hook for "gives us the chronik info and throws expected error on bad server connection"./chronik-info "before each" hook for "gives us the chronik info and throws expected error on bad server connection" ======
Error: Timeout of 2000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/work/modules/chronik-client/test/integration/chronik_info.ts)
    at listOnTimeout (node:internal/timers:559:17)
    at processTimers (node:internal/timers:502:7)

Each failure log is accessible here:
/chronik-info: "before each" hook for "gives us the chronik info and throws expected error on bad server connection"./chronik-info "before each" hook for "gives us the chronik info and throws expected error on bad server connection"

This timeout seems to occur when lint issues are present.

My workflow here is

arc diff
hit Y to the lint warnings
diff goes up, but lint warnings are in the diff
run arc lint locally, all good
push diff up again, lint warnings are gone, tests time out

not related to this diff specifically just an observation of a pattern from working on this one

bytesofman marked 4 inline comments as done.

add park tests, clean up py script, improve querystring logic now that we have default params

Failed tests logs:

====== /blockchain-info: "before each" hook for "gives us the blockchain info + throws expected error on bad connection"./blockchain-info "before each" hook for "gives us the blockchain info + throws expected error on bad connection" ======
Error: Timeout of 2000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/work/modules/chronik-client/test/integration/blockchain_info.ts)
    at listOnTimeout (node:internal/timers:559:17)
    at processTimers (node:internal/timers:502:7)

Each failure log is accessible here:
/blockchain-info: "before each" hook for "gives us the blockchain info + throws expected error on bad connection"./blockchain-info "before each" hook for "gives us the blockchain info + throws expected error on bad connection"

Fabien requested changes to this revision.Jan 31 2024, 09:10
Fabien added inline comments.
modules/chronik-client/src/ChronikClientNode.ts
61 โ†—(On Diff #44753)

This ternary makes no sense and is wrong. If the backend changes the default, say page size to 50, you'll get 50 tx if you ask 25.

This revision now requires changes to proceed.Jan 31 2024, 09:10

yield true not false for last step

Failed tests logs:

====== /block and /blocks: "before each" hook for "gives us the block and blocks after unparking the last block"./block and /blocks "before each" hook for "gives us the block and blocks after unparking the last block" ======
Error: Timeout of 2000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/work/modules/chronik-client/test/integration/block_and_blocks.ts)
    at listOnTimeout (node:internal/timers:559:17)
    at processTimers (node:internal/timers:502:7)

Each failure log is accessible here:
/block and /blocks: "before each" hook for "gives us the block and blocks after unparking the last block"./block and /blocks "before each" hook for "gives us the block and blocks after unparking the last block"

Failed tests logs:

====== /block and /blocks: "before each" hook for "gives us the block and blocks after unparking the last block"./block and /blocks "before each" hook for "gives us the block and blocks after unparking the last block" ======
Error: Timeout of 2000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/work/modules/chronik-client/test/integration/block_and_blocks.ts)
    at listOnTimeout (node:internal/timers:559:17)
    at processTimers (node:internal/timers:502:7)

Each failure log is accessible here:
/block and /blocks: "before each" hook for "gives us the block and blocks after unparking the last block"./block and /blocks "before each" hook for "gives us the block and blocks after unparking the last block"

D15342

This revision is now accepted and ready to land.Jan 31 2024, 13:13