Page MenuHomePhabricator
Differential D16627

[chronik client + ecash-herald + token-server + cashtab + ecash-lib + ecash-agora] Deprecate NNG support from chronik-client
ClosedPublic
Actions

Authored by bytesofman on Aug 12 2024, 18:02.

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Commits
rABCa110fd49527f: [chronik client + ecash-herald + token-server + cashtab + ecash-lib + ecash…
Summary

chronik was first developed on a bitcoin abc fork. It was later improved to work directly with the abc node in the monorepo.

This improved version was planned to be the only / default version.

Get rid of nng support. Rename all "InNode" types to lose this special marker. Rename ChronikClientNode to ChronikClient

Bump chronik-client to 1.0.0

Update ecash-herald, token-server, and Cashtab to handle this new type.

Note: alias-server and apps/examples are still legacy chronik nng apps. alias-server will be migrated to in-node if it is to be used for aliases. app/examples needs some thought. Potentially should be deprecated as the integration tests in chronik-client are more exhaustive, accurate, and easier to maintain.

I considered a phased approach where, say, we do not change all the names. I think though it is best to just drop this stuff, since there is no plan to maintain nng support going forward, and full migration to in-node was always the plan.

Test Plan

CI

To test existing implementations can still import and use ChronikClientNode

  • cd modules/chronik-client and npm run build
  • cd cashtab and npm start, send a tx, no API error
  • In cashtab/src/index.js, replace ChronikClient with ChronikClientNode
  • npm start, send a tx, no API error

Diff Detail

Event Timeline

bytesofman created this revision.Aug 12 2024, 18:02
Herald added a reviewer: Restricted Project. · View Herald TranscriptAug 12 2024, 18:02
Harbormaster completed remote builds in B29943: Diff 49155.Aug 12 2024, 18:04
teamcity added a comment.Aug 12 2024, 18:04
Build Bitcoin ABC Diffs / Diff Testing (token-server-tests) failed.

Tail of the build log:

   Compiling abc-rust-lint v0.1.0 (/work/chronik/abc-rust-lint)
   Compiling generic-array v0.14.7
   Compiling quote v1.0.36
   Compiling syn v2.0.72
   Compiling secp256k1-sys-abc v0.4.1 (https://github.com/raipay/secp256k1-abc?rev=b23e742#b23e7421)
   Compiling crypto-common v0.1.6
   Compiling block-buffer v0.10.4
   Compiling digest v0.10.7
   Compiling sha2 v0.10.8
   Compiling ripemd v0.1.3
   Compiling wasm-bindgen-backend v0.2.92
   Compiling secp256k1-abc v0.20.3 (https://github.com/raipay/secp256k1-abc?rev=b23e742#b23e7421)
   Compiling thiserror-impl v1.0.58
   Compiling wasm-bindgen-macro-support v0.2.92
   Compiling wasm-bindgen-macro v0.2.92
   Compiling ecash-lib-wasm v0.1.0 (/work/modules/ecash-lib-wasm)
    Finished release-wasm [optimized] target(s) in 7.17s
Test depends on chronik-client. Building TypeScript...
/work/modules/chronik-client /work/modules/ecash-lib-wasm /work/modules/mock-chronik-client /work/modules/ecashaddrjs /work/abc-ci-builds/token-server-tests

> chronik-client@0.29.0 prepublish
> npm run build


> chronik-client@0.29.0 build
> tsc


added 265 packages, and audited 267 packages in 6s

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

1 high severity vulnerability

To address all issues (including breaking changes), run:
  npm audit fix --force

Run `npm audit` for details.
Test depends on ecash-lib. Building TypeScript...
/work/modules/ecash-lib /work/modules/chronik-client /work/modules/ecash-lib-wasm /work/modules/mock-chronik-client /work/modules/ecashaddrjs /work/abc-ci-builds/token-server-tests

added 363 packages, and audited 366 packages in 2s

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

found 0 vulnerabilities

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

src/test/testRunner.ts(5,15): error TS2724: '"chronik-client"' has no exported member named 'ChronikClientNode'. Did you mean 'ChronikClient'?
src/test/testRunner.ts(44,17): error TS2339: Property 'ChronikClientNode' does not exist on type '{ default: typeof import("/work/modules/chronik-client/dist/index"); ChronikClient: typeof ChronikClient; ScriptEndpoint: typeof ScriptEndpoint; TokenIdEndpoint: typeof TokenIdEndpoint; LokadIdEndpoint: typeof LokadIdEndpoint; PluginEndpoint: typeof PluginEndpoint; WsEndpoint: typeof WsEndpoint; ALP_TOKEN_TYPES: Alp...'.
tests/alp.test.ts(7,10): error TS2724: '"chronik-client"' has no exported member named 'ChronikClientNode'. Did you mean 'ChronikClient'?
tests/slp.test.ts(6,10): error TS2724: '"chronik-client"' has no exported member named 'ChronikClientNode'. Did you mean 'ChronikClient'?
tests/slp.test.ts(444,24): error TS7006: Parameter 'msg' implicitly has an 'any' type.
tests/txBuilder.test.ts(7,10): error TS2724: '"chronik-client"' has no exported member named 'ChronikClientNode'. Did you mean 'ChronikClient'?
tests/txBuilder.test.ts(106,37): error TS7006: Parameter 'utxo' implicitly has an 'any' type.
Build token-server-tests failed with exit code 2
teamcity added a comment.Aug 12 2024, 18:04
Build Bitcoin ABC Diffs / Diff Testing (cashtab-tests) failed.

Tail of the build log:

  Downloaded wasm-bindgen-backend v0.2.92
  Downloaded wasm-bindgen v0.2.92
  Downloaded typenum v1.17.0
  Downloaded cc v1.0.92
  Downloaded thiserror-impl v1.0.58
  Downloaded thiserror v1.0.58
  Downloaded sha2 v0.10.8
  Downloaded ripemd v0.1.3
  Downloaded bumpalo v3.16.0
  Downloaded generic-array v0.14.7
  Downloaded cpufeatures v0.2.12
   Compiling proc-macro2 v1.0.86
   Compiling unicode-ident v1.0.12
   Compiling version_check v0.9.4
   Compiling typenum v1.17.0
   Compiling wasm-bindgen-shared v0.2.92
   Compiling cc v1.0.92
   Compiling bumpalo v3.16.0
   Compiling log v0.4.21
   Compiling once_cell v1.19.0
   Compiling thiserror v1.0.58
   Compiling cfg-if v1.0.0
   Compiling wasm-bindgen v0.2.92
   Compiling abc-rust-lint v0.1.0 (/work/chronik/abc-rust-lint)
   Compiling generic-array v0.14.7
   Compiling quote v1.0.36
   Compiling syn v2.0.72
   Compiling secp256k1-sys-abc v0.4.1 (https://github.com/raipay/secp256k1-abc?rev=b23e742#b23e7421)
   Compiling block-buffer v0.10.4
   Compiling crypto-common v0.1.6
   Compiling digest v0.10.7
   Compiling sha2 v0.10.8
   Compiling ripemd v0.1.3
   Compiling wasm-bindgen-backend v0.2.92
   Compiling secp256k1-abc v0.20.3 (https://github.com/raipay/secp256k1-abc?rev=b23e742#b23e7421)
   Compiling thiserror-impl v1.0.58
   Compiling wasm-bindgen-macro-support v0.2.92
   Compiling wasm-bindgen-macro v0.2.92
   Compiling ecash-lib-wasm v0.1.0 (/work/modules/ecash-lib-wasm)
    Finished release-wasm [optimized] target(s) in 8.14s
/work/modules/ecash-lib /work/modules/ecash-lib-wasm /work/modules/ecash-script /work/modules/chronik-client /work/modules/mock-chronik-client /work/modules/ecashaddrjs /work/abc-ci-builds/cashtab-tests

added 363 packages, and audited 366 packages in 2s

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

found 0 vulnerabilities

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

src/test/testRunner.ts(5,15): error TS2724: '"chronik-client"' has no exported member named 'ChronikClientNode'. Did you mean 'ChronikClient'?
src/test/testRunner.ts(44,17): error TS2339: Property 'ChronikClientNode' does not exist on type '{ default: typeof import("/work/modules/chronik-client/dist/index"); ChronikClient: typeof ChronikClient; ScriptEndpoint: typeof ScriptEndpoint; TokenIdEndpoint: typeof TokenIdEndpoint; LokadIdEndpoint: typeof LokadIdEndpoint; PluginEndpoint: typeof PluginEndpoint; WsEndpoint: typeof WsEndpoint; ALP_TOKEN_TYPES: Alp...'.
tests/alp.test.ts(7,10): error TS2724: '"chronik-client"' has no exported member named 'ChronikClientNode'. Did you mean 'ChronikClient'?
tests/slp.test.ts(6,10): error TS2724: '"chronik-client"' has no exported member named 'ChronikClientNode'. Did you mean 'ChronikClient'?
tests/slp.test.ts(444,24): error TS7006: Parameter 'msg' implicitly has an 'any' type.
tests/txBuilder.test.ts(7,10): error TS2724: '"chronik-client"' has no exported member named 'ChronikClientNode'. Did you mean 'ChronikClient'?
tests/txBuilder.test.ts(106,37): error TS7006: Parameter 'utxo' implicitly has an 'any' type.
Build cashtab-tests failed with exit code 2
teamcity added a comment.Aug 12 2024, 18:04
Build Bitcoin ABC Diffs / Diff Testing (ecash-lib-tests) failed.

Tail of the build log:

   Compiling syn v2.0.72
   Compiling secp256k1-sys-abc v0.4.1 (https://github.com/raipay/secp256k1-abc?rev=b23e742#b23e7421)
   Compiling block-buffer v0.10.4
   Compiling crypto-common v0.1.6
   Compiling digest v0.10.7
   Compiling sha2 v0.10.8
   Compiling ripemd v0.1.3
   Compiling wasm-bindgen-backend v0.2.92
   Compiling secp256k1-abc v0.20.3 (https://github.com/raipay/secp256k1-abc?rev=b23e742#b23e7421)
   Compiling thiserror-impl v1.0.58
   Compiling wasm-bindgen-macro-support v0.2.92
   Compiling wasm-bindgen-macro v0.2.92
   Compiling ecash-lib-wasm v0.1.0 (/work/modules/ecash-lib-wasm)
    Finished release-wasm [optimized] target(s) in 5.63s
Test depends on chronik-client. Building TypeScript...
/work/modules/chronik-client /work/modules/ecash-lib-wasm /work/modules/ecashaddrjs /work/abc-ci-builds/ecash-lib-tests

> chronik-client@0.29.0 prepublish
> npm run build


> chronik-client@0.29.0 build
> tsc


added 265 packages, and audited 267 packages in 5s

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

1 high severity vulnerability

To address all issues (including breaking changes), run:
  npm audit fix --force

Run `npm audit` for details.
Test does not depend on ecash-lib
Test does not depend on ecash-script, skipping ecash-script dependencies...
Test does not depend on ecash-coinselect, skipping ecash-coinselect dependencies...
/work/modules/ecash-lib /work/modules/chronik-client /work/modules/ecash-lib-wasm /work/modules/ecashaddrjs /work/abc-ci-builds/ecash-lib-tests

added 363 packages, and audited 366 packages in 2s

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

found 0 vulnerabilities
CI configured to test build. Building...

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

src/test/testRunner.ts(5,15): error TS2724: '"chronik-client"' has no exported member named 'ChronikClientNode'. Did you mean 'ChronikClient'?
src/test/testRunner.ts(44,17): error TS2339: Property 'ChronikClientNode' does not exist on type '{ default: typeof import("/work/modules/chronik-client/dist/index"); ChronikClient: typeof ChronikClient; ScriptEndpoint: typeof ScriptEndpoint; TokenIdEndpoint: typeof TokenIdEndpoint; LokadIdEndpoint: typeof LokadIdEndpoint; PluginEndpoint: typeof PluginEndpoint; WsEndpoint: typeof WsEndpoint; ALP_TOKEN_TYPES: Alp...'.
tests/alp.test.ts(7,10): error TS2724: '"chronik-client"' has no exported member named 'ChronikClientNode'. Did you mean 'ChronikClient'?
tests/slp.test.ts(6,10): error TS2724: '"chronik-client"' has no exported member named 'ChronikClientNode'. Did you mean 'ChronikClient'?
tests/slp.test.ts(444,24): error TS7006: Parameter 'msg' implicitly has an 'any' type.
tests/txBuilder.test.ts(7,10): error TS2724: '"chronik-client"' has no exported member named 'ChronikClientNode'. Did you mean 'ChronikClient'?
tests/txBuilder.test.ts(106,37): error TS7006: Parameter 'utxo' implicitly has an 'any' type.
Build ecash-lib-tests failed with exit code 2
bytesofman updated this revision to Diff 49156.Aug 12 2024, 18:10

update CI for ecash-lib, ecash-agora, update README with this diff for chronik-client

teamcity added a comment.Aug 12 2024, 18:11
The build failed due to an unexpected infrastructure outage. The administrators have been notified to investigate. Sorry for the inconvenience.
Harbormaster completed remote builds in B29944: Diff 49156.Aug 12 2024, 18:12
teamcity added a comment.Aug 12 2024, 18:12
Build Bitcoin ABC Diffs / Diff Testing (token-server-tests) failed.

Tail of the build log:

TSError: ⨯ Unable to compile TypeScript:
test/vectors.ts(7,5): error TS2305: Module '"chronik-client"' has no exported member 'TxInput_InNode'.
test/vectors.ts(8,5): error TS2305: Module '"chronik-client"' has no exported member 'TxOutput_InNode'.
test/vectors.ts(9,5): error TS2305: Module '"chronik-client"' has no exported member 'Token_InNode'.
test/vectors.ts(10,5): error TS2305: Module '"chronik-client"' has no exported member 'Tx_InNode'.
test/vectors.ts(11,5): error TS2305: Module '"chronik-client"' has no exported member 'BlockMetadata_InNode'.
test/vectors.ts(12,5): error TS2305: Module '"chronik-client"' has no exported member 'ScriptUtxo_InNode'.

    at createTSError (/work/apps/token-server/node_modules/ts-node/src/index.ts:859:12)
    at reportTSError (/work/apps/token-server/node_modules/ts-node/src/index.ts:863:19)
    at getOutput (/work/apps/token-server/node_modules/ts-node/src/index.ts:1077:36)
    at Object.compile (/work/apps/token-server/node_modules/ts-node/src/index.ts:1433:41)
    at Module.m._compile (/work/apps/token-server/node_modules/ts-node/src/index.ts:1617:30)
    at module.exports (/usr/lib/node_modules/nyc/node_modules/default-require-extensions/js.js:7:9)
    at /usr/lib/node_modules/nyc/node_modules/append-transform/index.js:64:4
    at require.extensions.<computed> (/work/apps/token-server/node_modules/ts-node/src/index.ts:1621:12)
    at Object.<anonymous> (/usr/lib/node_modules/nyc/node_modules/append-transform/index.js:64:4)
    at Module.load (node:internal/modules/cjs/loader:1208:32)
    at Function.Module._load (node:internal/modules/cjs/loader:1024:12)
    at Module.require (node:internal/modules/cjs/loader:1233:19)
    at require (node:internal/modules/helpers:179:18)
    at Object.<anonymous> (/work/apps/token-server/test/chronik/clientHandler.test.ts:8:1)
    at Module._compile (node:internal/modules/cjs/loader:1358:14)
    at Module.replacementCompile (/usr/lib/node_modules/nyc/node_modules/append-transform/index.js:60:13)
    at Module.m._compile (/work/apps/token-server/node_modules/ts-node/src/index.ts:1618:23)
    at module.exports (/usr/lib/node_modules/nyc/node_modules/default-require-extensions/js.js:7:9)
    at /usr/lib/node_modules/nyc/node_modules/append-transform/index.js:64:4
    at require.extensions.<computed> (/work/apps/token-server/node_modules/ts-node/src/index.ts:1621:12)
    at Object.<anonymous> (/usr/lib/node_modules/nyc/node_modules/append-transform/index.js:64:4)
    at Module.load (node:internal/modules/cjs/loader:1208:32)
    at Function.Module._load (node:internal/modules/cjs/loader:1024:12)
    at Module.require (node:internal/modules/cjs/loader:1233:19)
    at require (node:internal/modules/helpers:179:18)
    at Object.exports.requireOrImport (/work/apps/token-server/node_modules/mocha/lib/nodejs/esm-utils.js:53:16)
    at async Object.exports.loadFilesAsync (/work/apps/token-server/node_modules/mocha/lib/nodejs/esm-utils.js:100:20)
    at async singleRun (/work/apps/token-server/node_modules/mocha/lib/cli/run-helpers.js:125:3)
    at async Object.exports.handler (/work/apps/token-server/node_modules/mocha/lib/cli/run.js:370:5)
-------------------|---------|----------|---------|---------|-------------------
File               | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s 
-------------------|---------|----------|---------|---------|-------------------
All files          |   20.45 |     5.88 |       0 |   20.93 |                   
 scripts           |      60 |       50 |     100 |      60 |                   
  prepSecrets.ts   |      60 |       50 |     100 |      60 | 17-18             
 src/chronik       |   15.38 |        0 |       0 |   15.78 |                   
  clientHandler.ts |   11.76 |        0 |       0 |    12.5 | 27-85             
  parse.ts         |   18.18 |        0 |       0 |   18.18 | 26-32,47-69,78-93 
-------------------|---------|----------|---------|---------|-------------------

##teamcity[blockOpened name='Code Coverage Summary']
##teamcity[buildStatisticValue key='CodeCoverageAbsBCovered' value='9']
##teamcity[buildStatisticValue key='CodeCoverageAbsBTotal' value='44']
##teamcity[buildStatisticValue key='CodeCoverageAbsRCovered' value='1']
##teamcity[buildStatisticValue key='CodeCoverageAbsRTotal' value='17']
##teamcity[buildStatisticValue key='CodeCoverageAbsMCovered' value='0']
##teamcity[buildStatisticValue key='CodeCoverageAbsMTotal' value='5']
##teamcity[buildStatisticValue key='CodeCoverageAbsLCovered' value='9']
##teamcity[buildStatisticValue key='CodeCoverageAbsLTotal' value='43']
##teamcity[blockClosed name='Code Coverage Summary']
mv: cannot stat 'test_results/token-server-junit.xml': No such file or directory
Build token-server-tests failed with exit code 1
teamcity added a comment.Aug 12 2024, 18:12
Build Bitcoin ABC Diffs / Diff Testing (ecash-agora-integration-tests) failed.

Tail of the build log:

   Compiling abc-rust-lint v0.1.0 (/work/chronik/abc-rust-lint)
   Compiling generic-array v0.14.7
   Compiling quote v1.0.36
   Compiling syn v2.0.72
   Compiling secp256k1-sys-abc v0.4.1 (https://github.com/raipay/secp256k1-abc?rev=b23e742#b23e7421)
   Compiling block-buffer v0.10.4
   Compiling crypto-common v0.1.6
   Compiling digest v0.10.7
   Compiling ripemd v0.1.3
   Compiling sha2 v0.10.8
   Compiling wasm-bindgen-backend v0.2.92
   Compiling secp256k1-abc v0.20.3 (https://github.com/raipay/secp256k1-abc?rev=b23e742#b23e7421)
   Compiling thiserror-impl v1.0.58
   Compiling wasm-bindgen-macro-support v0.2.92
   Compiling wasm-bindgen-macro v0.2.92
   Compiling ecash-lib-wasm v0.1.0 (/work/modules/ecash-lib-wasm)
    Finished release-wasm [optimized] target(s) in 4.42s
Test depends on chronik-client. Building TypeScript...
/work/modules/chronik-client /work/modules/ecash-lib-wasm /work/modules/ecashaddrjs /work/abc-ci-builds/ecash-agora-integration-tests

> chronik-client@0.29.0 prepublish
> npm run build


> chronik-client@0.29.0 build
> tsc


added 265 packages, and audited 267 packages in 5s

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

1 high severity vulnerability

To address all issues (including breaking changes), run:
  npm audit fix --force

Run `npm audit` for details.
Test depends on ecash-lib. Building TypeScript...
/work/modules/ecash-lib /work/modules/chronik-client /work/modules/ecash-lib-wasm /work/modules/ecashaddrjs /work/abc-ci-builds/ecash-agora-integration-tests

added 363 packages, and audited 366 packages in 2s

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

found 0 vulnerabilities

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

src/test/testRunner.ts(5,15): error TS2724: '"chronik-client"' has no exported member named 'ChronikClientNode'. Did you mean 'ChronikClient'?
src/test/testRunner.ts(44,17): error TS2339: Property 'ChronikClientNode' does not exist on type '{ default: typeof import("/work/modules/chronik-client/dist/index"); ChronikClient: typeof ChronikClient; ScriptEndpoint: typeof ScriptEndpoint; TokenIdEndpoint: typeof TokenIdEndpoint; LokadIdEndpoint: typeof LokadIdEndpoint; PluginEndpoint: typeof PluginEndpoint; WsEndpoint: typeof WsEndpoint; ALP_TOKEN_TYPES: Alp...'.
tests/alp.test.ts(7,10): error TS2724: '"chronik-client"' has no exported member named 'ChronikClientNode'. Did you mean 'ChronikClient'?
tests/slp.test.ts(6,10): error TS2724: '"chronik-client"' has no exported member named 'ChronikClientNode'. Did you mean 'ChronikClient'?
tests/slp.test.ts(444,24): error TS7006: Parameter 'msg' implicitly has an 'any' type.
tests/txBuilder.test.ts(7,10): error TS2724: '"chronik-client"' has no exported member named 'ChronikClientNode'. Did you mean 'ChronikClient'?
tests/txBuilder.test.ts(106,37): error TS7006: Parameter 'utxo' implicitly has an 'any' type.
Build ecash-agora-integration-tests failed with exit code 2
bytesofman updated this revision to Diff 49157.Aug 12 2024, 18:32

updating types in token-server

Harbormaster completed remote builds in B29945: Diff 49157.Aug 12 2024, 18:43
bytesofman published this revision for review.Aug 12 2024, 18:46
bytesofman edited the summary of this revision. (Show Details)
bytesofman retitled this revision from [chronik client + ecash-herald + token-server + cashtab] Deprecate NNG support from chronik-client to [chronik client + ecash-herald + token-server + cashtab + ecash-lib + ecash-agora] Deprecate NNG support from chronik-client.
Fabien requested changes to this revision.Aug 13 2024, 07:07
Fabien added a subscriber: Fabien.

I mostly agree with the deprecation plan except for 1 thing: you should alias ChronikClientNode to ChronikClient. This makes it so that you don't need to update the libs that use ChronikClientNode (can be moved in their own diff). It also release pressure on devs to update as their current in-node chronik code will keep working with the new version.

As per the app examples they should be part of chronik-docs if anything. No opinion on the alias server as things are changing quite a bit.

This revision now requires changes to proceed.Aug 13 2024, 07:07
bytesofman updated this revision to Diff 49180.Aug 13 2024, 16:38

rebase, alias ChronikClient as ChronikClientNode to preserve existing implementations

bytesofman planned changes to this revision.Aug 13 2024, 16:39
bytesofman edited the test plan for this revision. (Show Details)

alias in this implementation actually breaks ChronikClient usage ... only ChronikClientNode works

bytesofman requested review of this revision.Aug 13 2024, 16:42

alias in this implementation actually breaks ChronikClient usage ... only ChronikClientNode works

never mind it works

I forgot to npm run build in chronik-client before testing

bytesofman edited the test plan for this revision. (Show Details)Aug 13 2024, 16:43
Harbormaster completed remote builds in B29966: Diff 49180.Aug 13 2024, 16:53
Fabien requested changes to this revision.Aug 13 2024, 19:57

Update ecash-herald, token-server, and Cashtab to handle this new type.

Because you aliased ChronikNodeClient this can (and should) be done in subsequent diffs.

This revision now requires changes to proceed.Aug 13 2024, 19:57
bytesofman updated this revision to Diff 49196.Aug 13 2024, 20:14
bytesofman edited the test plan for this revision. (Show Details)

rebase, back out ChronikClientNode to ChronikClient rename in monorepo apps

bytesofman added a comment.Aug 13 2024, 20:18
In D16627#376897, @Fabien wrote:

Update ecash-herald, token-server, and Cashtab to handle this new type.

Because you aliased ChronikNodeClient this can (and should) be done in subsequent diffs.

Backed out the ChronikClientNode to ChronikClient name change in associated apps -- however some code changes are still required due to changes in the type names.

...These could also be aliasesd but imo not worth it. I don't think keeping a handful of types clean is worth supporting "Everything has two names" for some indefinite period of time.

This change will be caught by linters by anyone who upgrades chronik-client.

Harbormaster completed remote builds in B29973: Diff 49196.Aug 13 2024, 20:39
Fabien requested changes to this revision.Aug 14 2024, 07:34
In D16627#376909, @bytesofman wrote:
In D16627#376897, @Fabien wrote:

Update ecash-herald, token-server, and Cashtab to handle this new type.

Because you aliased ChronikNodeClient this can (and should) be done in subsequent diffs.

Backed out the ChronikClientNode to ChronikClient name change in associated apps -- however some code changes are still required due to changes in the type names.

...These could also be aliasesd but imo not worth it. I don't think keeping a handful of types clean is worth supporting "Everything has two names" for some indefinite period of time.

This change will be caught by linters by anyone who upgrades chronik-client.

If you don't alias all the _InNode types then aliasing ChronikNodeClient is totally useless. Either do them all or none, and I think doing them all has a strong benefit for existing users. I agree we should note that it's deprecated in the release notes and will be removed in a future version.

This revision now requires changes to proceed.Aug 14 2024, 07:34
emack added a subscriber: emack.Aug 14 2024, 07:50

We probably need some comms on this along the lines of:

  1. next version is a breaking change for apps using NNG
  2. For existing in-node apps, refactor all references to ChronikClientNode to be ChronikClient
  3. For existing NNG powered apps, keep in mind changes a, b, c...... e.g. using address.history() going forward instead of script.history()
bytesofman requested review of this revision.EditedAug 14 2024, 16:15

I get the logic of "not doing the name change on every app in the monorepo at the same time" -- lets us easily back out diffs from one-app changes. Even with this though, I would rather just rip the bandaid off and get rid of it. Since we only have a handful of apps, and they are all linked to this repo in the monorepo, it's easy enough to test and confirm that the names are all updated in the monorepo.

will be removed in a future version.

If we're going to remove the legacy names in the future, we might as well just do it now. It's only going to make the transition worse down the line for any users with the old names / types.

The main driving force with this diff is to remove ambiguity between ChronikClient and ChronikClientNode. imo some confusion is inevitable and the longer we maintain dual nomenclature the longer we perpetuate the problem we are trying to solve. At some point, it will be a breaking change. Might as well be now when the number of apps is at its lowest.

Proposed steps here

  • I back out the version bump from this diff, so that landing does not publish a new version of chronik-client update: already done as there is no version bump implemented here
  • After this lands, 4 more diffs that implement ChronikClient instead of ChronikClientNode in ecash-herald, cashtab, token-server, and ecash-lib
  • Version bump 1.0.0 chronik-client diff that deprecates all legacy names and includes description of breaking change in the change log, this publishes the (breaking change) upgrade -- there are no new features in the upgrade
Fabien requested changes to this revision.Aug 15 2024, 07:54

Sum of nits, globally ok

apps/token-server/src/chronik/parse.ts
26 ↗(On Diff #49196)

just remove the useless comment entirely

cashtab/src/chronik/index.js
279 ↗(On Diff #49196)

unrelated but also weird change, the previous version makes more sense imo.
Please make sure we enforce a correct version of the linters so devs don't get different results based on the version

cashtab/src/components/Home/Tx/index.js
672 ↗(On Diff #49196)

same here, and other places

modules/chronik-client/README.md
13 ↗(On Diff #49196)

Remove. It's not necessary in this diff and also I don't think it's a good set of examples because their behavior rely on the python setup scripts, which makes it difficult to read for devs that are not familiar with the repo

21 ↗(On Diff #49196)

We should not use real urls here unless we have a public doc url available. Especially the pay2stay one which is not ours.

modules/chronik-client/src/failoverProxy.ts
294 ↗(On Diff #49196)

remove

modules/chronik-client/test/integration/broadcast_txs.ts
133 ↗(On Diff #49196)

remove

236 ↗(On Diff #49196)

dito. Comment should answer why not what, only exception being if it's a very complicated piece of code which is clearly not the case here

This revision now requires changes to proceed.Aug 15 2024, 07:54
bytesofman marked 8 inline comments as done.Aug 15 2024, 16:41
bytesofman added inline comments.
cashtab/src/chronik/index.js
279 ↗(On Diff #49196)

D16651

cashtab/src/components/Home/Tx/index.js
672 ↗(On Diff #49196)

D16651

bytesofman updated this revision to Diff 49234.Aug 15 2024, 16:41
bytesofman marked 2 inline comments as done.

remove useless comments, remove ref to tests in README, remove "real" server URLs from README

Harbormaster completed remote builds in B29997: Diff 49234.Aug 15 2024, 16:49
teamcity added a comment.Aug 15 2024, 16:49
Build Bitcoin ABC Diffs / Diff Testing (ecash-lib-integration-tests) failed.

Tail of the build log:

  File "/work/abc-ci-builds/ecash-lib-integration-tests/test/functional/test_runner.py", line 361, in main
    os.makedirs(tmpdir)
  File "/usr/lib/python3.9/os.py", line 225, in makedirs
    mkdir(name, mode)
FileExistsError: [Errno 17] File exists: '/work/abc-ci-builds/ecash-lib-integration-tests/test/tmp/test_runner_₿₵_🏃_20240815_164919'
Test runner completed with code 1
----------------------------|---------|----------|---------|---------|------------------------------
File                        | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s            
----------------------------|---------|----------|---------|---------|------------------------------
All files                   |   62.06 |    52.25 |   62.26 |   62.08 |                              
 ecash-lib                  |       0 |        0 |       0 |       0 |                              
  eslint.config.js          |       0 |        0 |       0 |       0 |                              
 ecash-lib/src              |   71.13 |    54.46 |   72.15 |   70.85 |                              
  ecc.ts                    |   57.14 |    83.33 |      40 |   57.14 | 23-31                        
  hash.ts                   |   88.88 |    83.33 |      80 |   88.88 | 14                           
  index.ts                  |       0 |        0 |       0 |       0 |                              
  indexBrowser.ts           |       0 |        0 |       0 |       0 |                              
  indexNodeJs.ts            |       0 |        0 |       0 |       0 |                              
  initBrowser.ts            |       0 |      100 |       0 |       0 | 11-13                        
  initNodeJs.ts             |     100 |      100 |     100 |     100 |                              
  op.ts                     |      40 |    44.44 |   66.66 |      40 | ...4,107,109,117-122,133-161 
  opcode.ts                 |     100 |    83.33 |     100 |     100 | 1                            
  script.ts                 |   52.63 |    38.09 |      60 |    50.9 | ...4-135,146,156,166,188-199 
  sigHashType.ts            |   77.77 |       44 |   85.71 |   77.77 | 26-38                        
  tx.ts                     |   93.47 |    79.16 |    90.9 |   93.18 | 123-125                      
  txBuilder.ts              |   52.74 |    46.42 |   66.66 |   51.72 | ...3-107,124-179,214,244-248 
  unsignedTx.ts             |    73.8 |    57.14 |   78.57 |   74.07 | ...3,151,159,184,192,198-201 
 ecash-lib/src/ffi          |   28.26 |    15.94 |   16.98 |   28.98 |                              
  ecash_lib_wasm_browser.js |       0 |        0 |       0 |       0 | 3-336                        
  ecash_lib_wasm_nodejs.js  |    61.9 |       55 |   39.13 |   62.75 | ...1,197-215,237,250-251,255 
 ecash-lib/src/io           |   59.55 |    60.29 |   70.58 |   58.77 |                              
  bytes.ts                  |     7.4 |    71.42 |    12.5 |     7.4 | 13-64                        
  hex.ts                    |   82.05 |     62.5 |      80 |   82.35 | 41-45,50,58                  
  int.ts                    |       0 |        0 |       0 |       0 |                              
  str.ts                    |   85.71 |    83.33 |   66.66 |   85.71 | 15                           
  varsize.ts                |      32 |    36.36 |   66.66 |      32 | 14-24,40-47                  
  writer.ts                 |       0 |        0 |       0 |       0 |                              
  writerbytes.ts            |   83.33 |    68.42 |     100 |   83.33 | 33,43,53,63,79               
  writerlength.ts           |     100 |    83.33 |     100 |     100 | 1                            
 ecash-lib/src/test         |   87.67 |    53.33 |    87.5 |   88.23 |                              
  testRunner.ts             |   87.67 |    53.33 |    87.5 |   88.23 | 69-71,84-85,108,119,162      
 ecash-lib/src/token        |   87.15 |    72.85 |   93.33 |   87.07 |                              
  alp.ts                    |   82.92 |    89.47 |   83.33 |   82.92 | 110-123,142                  
  common.ts                 |     100 |    83.33 |     100 |     100 | 1                            
  empp.ts                   |    92.3 |       75 |     100 |   91.66 | 12                           
  slp.ts                    |   89.74 |    62.16 |     100 |   89.74 | ...9,161,167,175,178,197,202 
----------------------------|---------|----------|---------|---------|------------------------------

##teamcity[blockOpened name='Code Coverage Summary']
##teamcity[buildStatisticValue key='CodeCoverageAbsBCovered' value='774']
##teamcity[buildStatisticValue key='CodeCoverageAbsBTotal' value='1247']
##teamcity[buildStatisticValue key='CodeCoverageAbsRCovered' value='243']
##teamcity[buildStatisticValue key='CodeCoverageAbsRTotal' value='465']
##teamcity[buildStatisticValue key='CodeCoverageAbsMCovered' value='132']
##teamcity[buildStatisticValue key='CodeCoverageAbsMTotal' value='212']
##teamcity[buildStatisticValue key='CodeCoverageAbsLCovered' value='755']
##teamcity[buildStatisticValue key='CodeCoverageAbsLTotal' value='1216']
##teamcity[blockClosed name='Code Coverage Summary']
mv: cannot stat 'test_results/ecash-lib-integration-tests-junit.xml': No such file or directory
Build ecash-lib-integration-tests failed with exit code 1
bytesofman added a comment.Aug 15 2024, 16:54

...have not seen this CI failure before and not able to diagnose the cause. re-running to see if it's a previously unseen flakiness. could be related to the rebase.

bytesofman added a comment.Aug 15 2024, 17:16

k -- not sure on the CI failure, but does not appear related to the cosmetic changes in this latest update.

Fabien accepted this revision.Aug 15 2024, 18:31
This revision is now accepted and ready to land.Aug 15 2024, 18:31
bytesofman updated this revision to Diff 49239.Aug 15 2024, 21:44

rebase

bytesofman updated this revision to Diff 49240.Aug 15 2024, 22:20

updating types from rebase

Harbormaster completed remote builds in B29999: Diff 49239.Aug 15 2024, 22:31
teamcity added a comment.Aug 15 2024, 22:31
Build Bitcoin ABC Diffs / Diff Testing (token-server-tests) failed.

Tail of the build log:

> token-server@0.0.0 test
> mocha --reporter mocha-junit-reporter --reporter-options mochaFile=test_results/token-server-junit.xml --reporter-options testsuitesTitle=Token Server Unit Tests --reporter-options rootSuiteTitle=Token Server


TSError: ⨯ Unable to compile TypeScript:
test/vectors.ts(308,12): error TS2304: Cannot find name 'ScriptUtxo_InNode'.
test/vectors.ts(317,20): error TS2304: Cannot find name 'ScriptUtxo_InNode'.

    at createTSError (/work/apps/token-server/node_modules/ts-node/src/index.ts:859:12)
    at reportTSError (/work/apps/token-server/node_modules/ts-node/src/index.ts:863:19)
    at getOutput (/work/apps/token-server/node_modules/ts-node/src/index.ts:1077:36)
    at Object.compile (/work/apps/token-server/node_modules/ts-node/src/index.ts:1433:41)
    at Module.m._compile (/work/apps/token-server/node_modules/ts-node/src/index.ts:1617:30)
    at module.exports (/usr/lib/node_modules/nyc/node_modules/default-require-extensions/js.js:7:9)
    at /usr/lib/node_modules/nyc/node_modules/append-transform/index.js:64:4
    at require.extensions.<computed> (/work/apps/token-server/node_modules/ts-node/src/index.ts:1621:12)
    at Object.<anonymous> (/usr/lib/node_modules/nyc/node_modules/append-transform/index.js:64:4)
    at Module.load (node:internal/modules/cjs/loader:1208:32)
    at Function.Module._load (node:internal/modules/cjs/loader:1024:12)
    at Module.require (node:internal/modules/cjs/loader:1233:19)
    at require (node:internal/modules/helpers:179:18)
    at Object.<anonymous> (/work/apps/token-server/test/chronik/clientHandler.test.ts:8:1)
    at Module._compile (node:internal/modules/cjs/loader:1358:14)
    at Module.replacementCompile (/usr/lib/node_modules/nyc/node_modules/append-transform/index.js:60:13)
    at Module.m._compile (/work/apps/token-server/node_modules/ts-node/src/index.ts:1618:23)
    at module.exports (/usr/lib/node_modules/nyc/node_modules/default-require-extensions/js.js:7:9)
    at /usr/lib/node_modules/nyc/node_modules/append-transform/index.js:64:4
    at require.extensions.<computed> (/work/apps/token-server/node_modules/ts-node/src/index.ts:1621:12)
    at Object.<anonymous> (/usr/lib/node_modules/nyc/node_modules/append-transform/index.js:64:4)
    at Module.load (node:internal/modules/cjs/loader:1208:32)
    at Function.Module._load (node:internal/modules/cjs/loader:1024:12)
    at Module.require (node:internal/modules/cjs/loader:1233:19)
    at require (node:internal/modules/helpers:179:18)
    at Object.exports.requireOrImport (/work/apps/token-server/node_modules/mocha/lib/nodejs/esm-utils.js:53:16)
    at async Object.exports.loadFilesAsync (/work/apps/token-server/node_modules/mocha/lib/nodejs/esm-utils.js:100:20)
    at async singleRun (/work/apps/token-server/node_modules/mocha/lib/cli/run-helpers.js:125:3)
    at async Object.exports.handler (/work/apps/token-server/node_modules/mocha/lib/cli/run.js:370:5)
-------------------|---------|----------|---------|---------|-------------------
File               | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s 
-------------------|---------|----------|---------|---------|-------------------
All files          |   20.45 |     5.88 |       0 |   20.93 |                   
 scripts           |      60 |       50 |     100 |      60 |                   
  prepSecrets.ts   |      60 |       50 |     100 |      60 | 17-18             
 src/chronik       |   15.38 |        0 |       0 |   15.78 |                   
  clientHandler.ts |   11.76 |        0 |       0 |    12.5 | 27-85             
  parse.ts         |   18.18 |        0 |       0 |   18.18 | 26-32,47-69,78-93 
-------------------|---------|----------|---------|---------|-------------------

##teamcity[blockOpened name='Code Coverage Summary']
##teamcity[buildStatisticValue key='CodeCoverageAbsBCovered' value='9']
##teamcity[buildStatisticValue key='CodeCoverageAbsBTotal' value='44']
##teamcity[buildStatisticValue key='CodeCoverageAbsRCovered' value='1']
##teamcity[buildStatisticValue key='CodeCoverageAbsRTotal' value='17']
##teamcity[buildStatisticValue key='CodeCoverageAbsMCovered' value='0']
##teamcity[buildStatisticValue key='CodeCoverageAbsMTotal' value='5']
##teamcity[buildStatisticValue key='CodeCoverageAbsLCovered' value='9']
##teamcity[buildStatisticValue key='CodeCoverageAbsLTotal' value='43']
##teamcity[blockClosed name='Code Coverage Summary']
mv: cannot stat 'test_results/token-server-junit.xml': No such file or directory
Build token-server-tests failed with exit code 1
Harbormaster completed remote builds in B30000: Diff 49240.Aug 15 2024, 22:47
Closed by commit rABCa110fd49527f: [chronik client + ecash-herald + token-server + cashtab + ecash-lib + ecash… (authored by bytesofman). · Explain WhyAug 15 2024, 22:57
This revision was automatically updated to reflect the committed changes.
bytesofman added a commit: rABCa110fd49527f: [chronik client + ecash-herald + token-server + cashtab + ecash-lib + ecash….
bytesofman mentioned this in D16544: [ecash-agora] Add `agora.py` plugin to index ONESHOT offers.Aug 15 2024, 23:00

Revision Contents
Changeset List

PathSize
apps/
token-server/
src/
chronik/
10 lines
12 lines
4 lines
8 lines
4 lines
test/
120 lines
cashtab/
src/
chronik/
16 lines
slpv1/
8 lines
modules/
chronik-client/
43 lines
1 line
2 lines
proto/
6752 lines
src/
2024 lines
83 lines
2 lines
test/
integration/
20 lines
16 lines
20 lines
24 lines
8 lines
12 lines
14 lines
18 lines
10 lines
74 lines
60 lines
29 lines
23 lines
29 lines
10 lines
ecash-agora/
src/
2 lines

Diff 49239

View Options

apps/token-server/src/chronik/clientHandler.ts

Loading...
View Options

apps/token-server/src/chronik/parse.ts

Loading...
View Options

apps/token-server/src/rewards.ts

Loading...
View Options

apps/token-server/src/transactions.ts

Loading...
View Options

apps/token-server/src/wallet.ts

Loading...
View Options

apps/token-server/test/vectors.ts

Loading...
View Options

cashtab/src/chronik/index.js

Loading...
View Options

cashtab/src/slpv1/index.js

Loading...
View Options

modules/chronik-client/README.md

Loading...
View Options

modules/chronik-client/index.ts

Loading...
View Options

modules/chronik-client/package.json

Loading...
View Options

modules/chronik-client/proto/chronik.ts

Loading...
View Options

modules/chronik-client/proto/chronikNode.ts

Loading...
View Options

modules/chronik-client/src/ChronikClient.ts

Loading...
View Options

modules/chronik-client/src/ChronikClientNode.ts

Loading...
View Options

modules/chronik-client/src/failoverProxy.ts

Loading...
View Options

modules/chronik-client/src/validation.ts

Loading...
View Options

modules/chronik-client/test/integration/block_and_blocks.ts

Loading...
View Options

modules/chronik-client/test/integration/blockchain_info.ts

Loading...
View Options

modules/chronik-client/test/integration/blocktxs_and_tx_and_rawtx.ts

Loading...
View Options

modules/chronik-client/test/integration/broadcast_txs.ts

Loading...
View Options

modules/chronik-client/test/integration/chronik_info.ts

Loading...
View Options

modules/chronik-client/test/integration/lokad_id.ts

Loading...
View Options

modules/chronik-client/test/integration/mempool_conflicts.ts

Loading...
View Options

modules/chronik-client/test/integration/plugin_groups.ts

Loading...
View Options

modules/chronik-client/test/integration/plugins.ts

Loading...
View Options

modules/chronik-client/test/integration/script_endpoints.ts

Loading...
View Options

modules/chronik-client/test/integration/token_alp.ts

Loading...
View Options

modules/chronik-client/test/integration/token_slp_fungible.ts

Loading...
View Options

modules/chronik-client/test/integration/token_slp_mint_vault.ts

Loading...
View Options

modules/chronik-client/test/integration/token_slp_nft1.ts

Loading...
View Options

modules/chronik-client/test/integration/websocket.ts

Loading...
View Options

modules/ecash-agora/src/ad.ts

Loading...