Page MenuHomePhabricator
Differential D16171

[ecash-herald, mock-chronik-client, alias-server] Migrate ecash-herald to in-node chronik-client and add supporting mock-chronik-client methods
ClosedPublic
Actions

Authored by bytesofman on May 15 2024, 22:50.

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Commits
rABC9abe1c8ba966: [ecash-herald, mock-chronik-client, alias-server] Migrate ecash-herald to in…
Summary

Migrate ecash-herald to in-node chronik-client

This diff is intended to maintain existing functionality of existing ecash-herald with as little refactoring as possible. New in-node chronik-client allows for many improvements to ecash-herald that will be handled in other refactors.

ALP txs now appear under "eToken sends" and not EMPP

Currently, we can detect a block that is found but rejected by avalanche bc the herald will send an error msg -- the block connects, but is not indexed, so the herald's chronik.block call fails and we see a default msg. ABC devs get what is going on but this is not clear to users.

I've added caching to preserve this alert feature. When a block is connected, ecash-herald will wait 10s. If no finalized msg comes in, a "block connected but not finalized by avalanche for 10s" is sent.

I added unit tests for this behavior and also got the msg to send testing prod by waiting 10ms instead of 10s:

image.png (201×644 px, 27 KB)

In practical testing, the BLK_FINALIZED msg seems to come in only a few seconds after the BLK_CONNECTED msg.

Some adjustments are made to alias-server unit tests because the mock-chronik-client change breaks them. Imo this removal is justified because

  • the behavior tested in alias-server (ws subscription) is not important to test...really it's testing the dependency.
  • alias-server will need to implement in-node chronik if alias path fwd still requires its use. in this case, it will need the mock-chronik-client changes introduced in this diff, just like the herald did.
  • does not make sense to implement in-node chronik-client in alias-server in this diff just to preserve these tests, the behavior change is expected. implementing in-node chronik-client in alias-server should be its own diff.
Test Plan

npm test, CI for mock-chronik-client

Test block msg from nng:

image.png (769×634 px, 113 KB)

image.png (613×634 px, 141 KB)

Test block msg now:

image.png (740×613 px, 97 KB)

image.png (655×637 px, 157 KB)

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
Harbormaster completed remote builds in B29021: Diff 47803.May 15 2024, 22:58
bytesofman updated this revision to Diff 47900.Wed, May 22, 18:36

better token parsing

Harbormaster completed remote builds in B29085: Diff 47900.Wed, May 22, 18:36
bytesofman updated this revision to Diff 47901.Wed, May 22, 18:37

lint

Harbormaster completed remote builds in B29086: Diff 47901.Wed, May 22, 18:40
bytesofman updated this revision to Diff 47904.Wed, May 22, 20:27

update mock syntax to match in-node chronik, add token genesisinfo to block mock, remove debug logs

Harbormaster completed remote builds in B29088: Diff 47904.Wed, May 22, 20:31
bytesofman updated this revision to Diff 47905.Wed, May 22, 22:00

update tests

Harbormaster completed remote builds in B29089: Diff 47905.Wed, May 22, 22:04
bytesofman updated this revision to Diff 47906.Wed, May 22, 22:16

update mock-chronik-client for in-node ws subs, update herald for correct in-node ws subscription

Harbormaster completed remote builds in B29090: Diff 47906.Wed, May 22, 22:16
bytesofman updated this revision to Diff 47907.Wed, May 22, 22:17

lint

teamcity added a comment.Wed, May 22, 22:18
Build Bitcoin ABC Diffs / Diff Testing (alias-server-tests) failed.

Failed tests logs:

====== alias-server chronikWsHandler.js: initializeWebsocket returns expected websocket object for a p2pkh address.alias-server chronikWsHandler.js initializeWebsocket returns expected websocket object for a p2pkh address ======
AssertionError: Expected values to be loosely deep-equal:  {   blocks: false,   lokadIds: [],   scripts: [     {       scriptPayload: '638568e36d0b5d7d49a6e99854caa27d9772b093',       scriptType: 'p2pkh'     }   ],   tokens: [] }  should loosely deep-equal  [   {     scriptPayload: '638568e36d0b5d7d49a6e99854caa27d9772b093',     scriptType: 'p2pkh'   } ]
AssertionError [ERR_ASSERTION]: Expected values to be loosely deep-equal:

{
  blocks: false,
  lokadIds: [],
  scripts: [
    {
      scriptPayload: '638568e36d0b5d7d49a6e99854caa27d9772b093',
      scriptType: 'p2pkh'
    }
  ],
  tokens: []
}

should loosely deep-equal

[
  {
    scriptPayload: '638568e36d0b5d7d49a6e99854caa27d9772b093',
    scriptType: 'p2pkh'
  }
]
    at Context.<anonymous> (test/chronikWsHandler.test.js:110:16)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)

      + expected - actual:  failed to generate Mocha diff
====== alias-server chronikWsHandler.js: initializeWebsocket returns expected websocket object for a p2sh address.alias-server chronikWsHandler.js initializeWebsocket returns expected websocket object for a p2sh address ======
AssertionError: Expected values to be loosely deep-equal:  {   blocks: false,   lokadIds: [],   scripts: [     {       scriptPayload: 'd37c4c809fe9840e7bfa77b86bd47163f6fb6c60',       scriptType: 'p2sh'     }   ],   tokens: [] }  should loosely deep-equal  [   {     scriptPayload: 'd37c4c809fe9840e7bfa77b86bd47163f6fb6c60',     scriptType: 'p2sh'   } ]
AssertionError [ERR_ASSERTION]: Expected values to be loosely deep-equal:

{
  blocks: false,
  lokadIds: [],
  scripts: [
    {
      scriptPayload: 'd37c4c809fe9840e7bfa77b86bd47163f6fb6c60',
      scriptType: 'p2sh'
    }
  ],
  tokens: []
}

should loosely deep-equal

[
  {
    scriptPayload: 'd37c4c809fe9840e7bfa77b86bd47163f6fb6c60',
    scriptType: 'p2sh'
  }
]
    at Context.<anonymous> (test/chronikWsHandler.test.js:140:16)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)

      + expected - actual:  failed to generate Mocha diff
====== alias-server main.js: main() connects to a websocket, and runs handleAppStartup() correctly.alias-server main.js main() connects to a websocket, and runs handleAppStartup() correctly ======
AssertionError: Expected values to be loosely deep-equal:  {   blocks: false,   lokadIds: [],   scripts: [     {       scriptPayload: 'd37c4c809fe9840e7bfa77b86bd47163f6fb6c60',       scriptType: 'p2sh'     }   ],   tokens: [] }  should loosely deep-equal  [   {     scriptPayload: 'd37c4c809fe9840e7bfa77b86bd47163f6fb6c60',     scriptType: 'p2sh'   } ]
AssertionError [ERR_ASSERTION]: Expected values to be loosely deep-equal:

{
  blocks: false,
  lokadIds: [],
  scripts: [
    {
      scriptPayload: 'd37c4c809fe9840e7bfa77b86bd47163f6fb6c60',
      scriptType: 'p2sh'
    }
  ],
  tokens: []
}

should loosely deep-equal

[
  {
    scriptPayload: 'd37c4c809fe9840e7bfa77b86bd47163f6fb6c60',
    scriptType: 'p2sh'
  }
]
    at Context.<anonymous> (test/main.test.js:117:16)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)

      + expected - actual:  failed to generate Mocha diff

Each failure log is accessible here:
alias-server chronikWsHandler.js: initializeWebsocket returns expected websocket object for a p2pkh address.alias-server chronikWsHandler.js initializeWebsocket returns expected websocket object for a p2pkh address
alias-server chronikWsHandler.js: initializeWebsocket returns expected websocket object for a p2sh address.alias-server chronikWsHandler.js initializeWebsocket returns expected websocket object for a p2sh address
alias-server main.js: main() connects to a websocket, and runs handleAppStartup() correctly.alias-server main.js main() connects to a websocket, and runs handleAppStartup() correctly

Harbormaster completed remote builds in B29091: Diff 47907.Wed, May 22, 22:19
teamcity added a comment.Wed, May 22, 22:19
Build Bitcoin ABC Diffs / Diff Testing (alias-server-tests) failed.

Failed tests logs:

====== alias-server chronikWsHandler.js: initializeWebsocket returns expected websocket object for a p2pkh address.alias-server chronikWsHandler.js initializeWebsocket returns expected websocket object for a p2pkh address ======
AssertionError: Expected values to be loosely deep-equal:  {   blocks: false,   lokadIds: [],   scripts: [     {       scriptPayload: '638568e36d0b5d7d49a6e99854caa27d9772b093',       scriptType: 'p2pkh'     }   ],   tokens: [] }  should loosely deep-equal  [   {     scriptPayload: '638568e36d0b5d7d49a6e99854caa27d9772b093',     scriptType: 'p2pkh'   } ]
AssertionError [ERR_ASSERTION]: Expected values to be loosely deep-equal:

{
  blocks: false,
  lokadIds: [],
  scripts: [
    {
      scriptPayload: '638568e36d0b5d7d49a6e99854caa27d9772b093',
      scriptType: 'p2pkh'
    }
  ],
  tokens: []
}

should loosely deep-equal

[
  {
    scriptPayload: '638568e36d0b5d7d49a6e99854caa27d9772b093',
    scriptType: 'p2pkh'
  }
]
    at Context.<anonymous> (test/chronikWsHandler.test.js:110:16)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)

      + expected - actual:  failed to generate Mocha diff
====== alias-server chronikWsHandler.js: initializeWebsocket returns expected websocket object for a p2sh address.alias-server chronikWsHandler.js initializeWebsocket returns expected websocket object for a p2sh address ======
AssertionError: Expected values to be loosely deep-equal:  {   blocks: false,   lokadIds: [],   scripts: [     {       scriptPayload: 'd37c4c809fe9840e7bfa77b86bd47163f6fb6c60',       scriptType: 'p2sh'     }   ],   tokens: [] }  should loosely deep-equal  [   {     scriptPayload: 'd37c4c809fe9840e7bfa77b86bd47163f6fb6c60',     scriptType: 'p2sh'   } ]
AssertionError [ERR_ASSERTION]: Expected values to be loosely deep-equal:

{
  blocks: false,
  lokadIds: [],
  scripts: [
    {
      scriptPayload: 'd37c4c809fe9840e7bfa77b86bd47163f6fb6c60',
      scriptType: 'p2sh'
    }
  ],
  tokens: []
}

should loosely deep-equal

[
  {
    scriptPayload: 'd37c4c809fe9840e7bfa77b86bd47163f6fb6c60',
    scriptType: 'p2sh'
  }
]
    at Context.<anonymous> (test/chronikWsHandler.test.js:140:16)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)

      + expected - actual:  failed to generate Mocha diff
====== alias-server main.js: main() connects to a websocket, and runs handleAppStartup() correctly.alias-server main.js main() connects to a websocket, and runs handleAppStartup() correctly ======
AssertionError: Expected values to be loosely deep-equal:  {   blocks: false,   lokadIds: [],   scripts: [     {       scriptPayload: 'd37c4c809fe9840e7bfa77b86bd47163f6fb6c60',       scriptType: 'p2sh'     }   ],   tokens: [] }  should loosely deep-equal  [   {     scriptPayload: 'd37c4c809fe9840e7bfa77b86bd47163f6fb6c60',     scriptType: 'p2sh'   } ]
AssertionError [ERR_ASSERTION]: Expected values to be loosely deep-equal:

{
  blocks: false,
  lokadIds: [],
  scripts: [
    {
      scriptPayload: 'd37c4c809fe9840e7bfa77b86bd47163f6fb6c60',
      scriptType: 'p2sh'
    }
  ],
  tokens: []
}

should loosely deep-equal

[
  {
    scriptPayload: 'd37c4c809fe9840e7bfa77b86bd47163f6fb6c60',
    scriptType: 'p2sh'
  }
]
    at Context.<anonymous> (test/main.test.js:117:16)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)

      + expected - actual:  failed to generate Mocha diff

Each failure log is accessible here:
alias-server chronikWsHandler.js: initializeWebsocket returns expected websocket object for a p2pkh address.alias-server chronikWsHandler.js initializeWebsocket returns expected websocket object for a p2pkh address
alias-server chronikWsHandler.js: initializeWebsocket returns expected websocket object for a p2sh address.alias-server chronikWsHandler.js initializeWebsocket returns expected websocket object for a p2sh address
alias-server main.js: main() connects to a websocket, and runs handleAppStartup() correctly.alias-server main.js main() connects to a websocket, and runs handleAppStartup() correctly

bytesofman updated this revision to Diff 47908.Wed, May 22, 22:30

patch script to also accept cache param

Harbormaster completed remote builds in B29092: Diff 47908.Wed, May 22, 22:31
teamcity added a comment.Wed, May 22, 22:31
Build Bitcoin ABC Diffs / Diff Testing (alias-server-tests) failed.

Failed tests logs:

====== alias-server chronikWsHandler.js: initializeWebsocket returns expected websocket object for a p2pkh address.alias-server chronikWsHandler.js initializeWebsocket returns expected websocket object for a p2pkh address ======
AssertionError: Expected values to be loosely deep-equal:  {   blocks: false,   lokadIds: [],   scripts: [     {       scriptPayload: '638568e36d0b5d7d49a6e99854caa27d9772b093',       scriptType: 'p2pkh'     }   ],   tokens: [] }  should loosely deep-equal  [   {     scriptPayload: '638568e36d0b5d7d49a6e99854caa27d9772b093',     scriptType: 'p2pkh'   } ]
AssertionError [ERR_ASSERTION]: Expected values to be loosely deep-equal:

{
  blocks: false,
  lokadIds: [],
  scripts: [
    {
      scriptPayload: '638568e36d0b5d7d49a6e99854caa27d9772b093',
      scriptType: 'p2pkh'
    }
  ],
  tokens: []
}

should loosely deep-equal

[
  {
    scriptPayload: '638568e36d0b5d7d49a6e99854caa27d9772b093',
    scriptType: 'p2pkh'
  }
]
    at Context.<anonymous> (test/chronikWsHandler.test.js:110:16)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)

      + expected - actual:  failed to generate Mocha diff
====== alias-server chronikWsHandler.js: initializeWebsocket returns expected websocket object for a p2sh address.alias-server chronikWsHandler.js initializeWebsocket returns expected websocket object for a p2sh address ======
AssertionError: Expected values to be loosely deep-equal:  {   blocks: false,   lokadIds: [],   scripts: [     {       scriptPayload: 'd37c4c809fe9840e7bfa77b86bd47163f6fb6c60',       scriptType: 'p2sh'     }   ],   tokens: [] }  should loosely deep-equal  [   {     scriptPayload: 'd37c4c809fe9840e7bfa77b86bd47163f6fb6c60',     scriptType: 'p2sh'   } ]
AssertionError [ERR_ASSERTION]: Expected values to be loosely deep-equal:

{
  blocks: false,
  lokadIds: [],
  scripts: [
    {
      scriptPayload: 'd37c4c809fe9840e7bfa77b86bd47163f6fb6c60',
      scriptType: 'p2sh'
    }
  ],
  tokens: []
}

should loosely deep-equal

[
  {
    scriptPayload: 'd37c4c809fe9840e7bfa77b86bd47163f6fb6c60',
    scriptType: 'p2sh'
  }
]
    at Context.<anonymous> (test/chronikWsHandler.test.js:140:16)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)

      + expected - actual:  failed to generate Mocha diff
====== alias-server main.js: main() connects to a websocket, and runs handleAppStartup() correctly.alias-server main.js main() connects to a websocket, and runs handleAppStartup() correctly ======
AssertionError: Expected values to be loosely deep-equal:  {   blocks: false,   lokadIds: [],   scripts: [     {       scriptPayload: 'd37c4c809fe9840e7bfa77b86bd47163f6fb6c60',       scriptType: 'p2sh'     }   ],   tokens: [] }  should loosely deep-equal  [   {     scriptPayload: 'd37c4c809fe9840e7bfa77b86bd47163f6fb6c60',     scriptType: 'p2sh'   } ]
AssertionError [ERR_ASSERTION]: Expected values to be loosely deep-equal:

{
  blocks: false,
  lokadIds: [],
  scripts: [
    {
      scriptPayload: 'd37c4c809fe9840e7bfa77b86bd47163f6fb6c60',
      scriptType: 'p2sh'
    }
  ],
  tokens: []
}

should loosely deep-equal

[
  {
    scriptPayload: 'd37c4c809fe9840e7bfa77b86bd47163f6fb6c60',
    scriptType: 'p2sh'
  }
]
    at Context.<anonymous> (test/main.test.js:117:16)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)

      + expected - actual:  failed to generate Mocha diff

Each failure log is accessible here:
alias-server chronikWsHandler.js: initializeWebsocket returns expected websocket object for a p2pkh address.alias-server chronikWsHandler.js initializeWebsocket returns expected websocket object for a p2pkh address
alias-server chronikWsHandler.js: initializeWebsocket returns expected websocket object for a p2sh address.alias-server chronikWsHandler.js initializeWebsocket returns expected websocket object for a p2sh address
alias-server main.js: main() connects to a websocket, and runs handleAppStartup() correctly.alias-server main.js main() connects to a websocket, and runs handleAppStartup() correctly

bytesofman updated this revision to Diff 47909.Wed, May 22, 22:33

call main with correct params

Harbormaster completed remote builds in B29093: Diff 47909.Wed, May 22, 22:35
teamcity added a comment.Wed, May 22, 22:35
Build Bitcoin ABC Diffs / Diff Testing (alias-server-tests) failed.

Failed tests logs:

====== alias-server chronikWsHandler.js: initializeWebsocket returns expected websocket object for a p2pkh address.alias-server chronikWsHandler.js initializeWebsocket returns expected websocket object for a p2pkh address ======
AssertionError: Expected values to be loosely deep-equal:  {   blocks: false,   lokadIds: [],   scripts: [     {       scriptPayload: '638568e36d0b5d7d49a6e99854caa27d9772b093',       scriptType: 'p2pkh'     }   ],   tokens: [] }  should loosely deep-equal  [   {     scriptPayload: '638568e36d0b5d7d49a6e99854caa27d9772b093',     scriptType: 'p2pkh'   } ]
AssertionError [ERR_ASSERTION]: Expected values to be loosely deep-equal:

{
  blocks: false,
  lokadIds: [],
  scripts: [
    {
      scriptPayload: '638568e36d0b5d7d49a6e99854caa27d9772b093',
      scriptType: 'p2pkh'
    }
  ],
  tokens: []
}

should loosely deep-equal

[
  {
    scriptPayload: '638568e36d0b5d7d49a6e99854caa27d9772b093',
    scriptType: 'p2pkh'
  }
]
    at Context.<anonymous> (test/chronikWsHandler.test.js:110:16)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)

      + expected - actual:  failed to generate Mocha diff
====== alias-server chronikWsHandler.js: initializeWebsocket returns expected websocket object for a p2sh address.alias-server chronikWsHandler.js initializeWebsocket returns expected websocket object for a p2sh address ======
AssertionError: Expected values to be loosely deep-equal:  {   blocks: false,   lokadIds: [],   scripts: [     {       scriptPayload: 'd37c4c809fe9840e7bfa77b86bd47163f6fb6c60',       scriptType: 'p2sh'     }   ],   tokens: [] }  should loosely deep-equal  [   {     scriptPayload: 'd37c4c809fe9840e7bfa77b86bd47163f6fb6c60',     scriptType: 'p2sh'   } ]
AssertionError [ERR_ASSERTION]: Expected values to be loosely deep-equal:

{
  blocks: false,
  lokadIds: [],
  scripts: [
    {
      scriptPayload: 'd37c4c809fe9840e7bfa77b86bd47163f6fb6c60',
      scriptType: 'p2sh'
    }
  ],
  tokens: []
}

should loosely deep-equal

[
  {
    scriptPayload: 'd37c4c809fe9840e7bfa77b86bd47163f6fb6c60',
    scriptType: 'p2sh'
  }
]
    at Context.<anonymous> (test/chronikWsHandler.test.js:140:16)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)

      + expected - actual:  failed to generate Mocha diff
====== alias-server main.js: main() connects to a websocket, and runs handleAppStartup() correctly.alias-server main.js main() connects to a websocket, and runs handleAppStartup() correctly ======
AssertionError: Expected values to be loosely deep-equal:  {   blocks: false,   lokadIds: [],   scripts: [     {       scriptPayload: 'd37c4c809fe9840e7bfa77b86bd47163f6fb6c60',       scriptType: 'p2sh'     }   ],   tokens: [] }  should loosely deep-equal  [   {     scriptPayload: 'd37c4c809fe9840e7bfa77b86bd47163f6fb6c60',     scriptType: 'p2sh'   } ]
AssertionError [ERR_ASSERTION]: Expected values to be loosely deep-equal:

{
  blocks: false,
  lokadIds: [],
  scripts: [
    {
      scriptPayload: 'd37c4c809fe9840e7bfa77b86bd47163f6fb6c60',
      scriptType: 'p2sh'
    }
  ],
  tokens: []
}

should loosely deep-equal

[
  {
    scriptPayload: 'd37c4c809fe9840e7bfa77b86bd47163f6fb6c60',
    scriptType: 'p2sh'
  }
]
    at Context.<anonymous> (test/main.test.js:117:16)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)

      + expected - actual:  failed to generate Mocha diff

Each failure log is accessible here:
alias-server chronikWsHandler.js: initializeWebsocket returns expected websocket object for a p2pkh address.alias-server chronikWsHandler.js initializeWebsocket returns expected websocket object for a p2pkh address
alias-server chronikWsHandler.js: initializeWebsocket returns expected websocket object for a p2sh address.alias-server chronikWsHandler.js initializeWebsocket returns expected websocket object for a p2sh address
alias-server main.js: main() connects to a websocket, and runs handleAppStartup() correctly.alias-server main.js main() connects to a websocket, and runs handleAppStartup() correctly

bytesofman updated this revision to Diff 47910.Wed, May 22, 23:08

add tests for the case of unfinalized blocks

Harbormaster completed remote builds in B29094: Diff 47910.Wed, May 22, 23:10
teamcity added a comment.Wed, May 22, 23:10
Build Bitcoin ABC Diffs / Diff Testing (alias-server-tests) failed.

Failed tests logs:

====== alias-server chronikWsHandler.js: initializeWebsocket returns expected websocket object for a p2pkh address.alias-server chronikWsHandler.js initializeWebsocket returns expected websocket object for a p2pkh address ======
AssertionError: Expected values to be loosely deep-equal:  {   blocks: false,   lokadIds: [],   scripts: [     {       scriptPayload: '638568e36d0b5d7d49a6e99854caa27d9772b093',       scriptType: 'p2pkh'     }   ],   tokens: [] }  should loosely deep-equal  [   {     scriptPayload: '638568e36d0b5d7d49a6e99854caa27d9772b093',     scriptType: 'p2pkh'   } ]
AssertionError [ERR_ASSERTION]: Expected values to be loosely deep-equal:

{
  blocks: false,
  lokadIds: [],
  scripts: [
    {
      scriptPayload: '638568e36d0b5d7d49a6e99854caa27d9772b093',
      scriptType: 'p2pkh'
    }
  ],
  tokens: []
}

should loosely deep-equal

[
  {
    scriptPayload: '638568e36d0b5d7d49a6e99854caa27d9772b093',
    scriptType: 'p2pkh'
  }
]
    at Context.<anonymous> (test/chronikWsHandler.test.js:110:16)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)

      + expected - actual:  failed to generate Mocha diff
====== alias-server chronikWsHandler.js: initializeWebsocket returns expected websocket object for a p2sh address.alias-server chronikWsHandler.js initializeWebsocket returns expected websocket object for a p2sh address ======
AssertionError: Expected values to be loosely deep-equal:  {   blocks: false,   lokadIds: [],   scripts: [     {       scriptPayload: 'd37c4c809fe9840e7bfa77b86bd47163f6fb6c60',       scriptType: 'p2sh'     }   ],   tokens: [] }  should loosely deep-equal  [   {     scriptPayload: 'd37c4c809fe9840e7bfa77b86bd47163f6fb6c60',     scriptType: 'p2sh'   } ]
AssertionError [ERR_ASSERTION]: Expected values to be loosely deep-equal:

{
  blocks: false,
  lokadIds: [],
  scripts: [
    {
      scriptPayload: 'd37c4c809fe9840e7bfa77b86bd47163f6fb6c60',
      scriptType: 'p2sh'
    }
  ],
  tokens: []
}

should loosely deep-equal

[
  {
    scriptPayload: 'd37c4c809fe9840e7bfa77b86bd47163f6fb6c60',
    scriptType: 'p2sh'
  }
]
    at Context.<anonymous> (test/chronikWsHandler.test.js:140:16)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)

      + expected - actual:  failed to generate Mocha diff
====== alias-server main.js: main() connects to a websocket, and runs handleAppStartup() correctly.alias-server main.js main() connects to a websocket, and runs handleAppStartup() correctly ======
AssertionError: Expected values to be loosely deep-equal:  {   blocks: false,   lokadIds: [],   scripts: [     {       scriptPayload: 'd37c4c809fe9840e7bfa77b86bd47163f6fb6c60',       scriptType: 'p2sh'     }   ],   tokens: [] }  should loosely deep-equal  [   {     scriptPayload: 'd37c4c809fe9840e7bfa77b86bd47163f6fb6c60',     scriptType: 'p2sh'   } ]
AssertionError [ERR_ASSERTION]: Expected values to be loosely deep-equal:

{
  blocks: false,
  lokadIds: [],
  scripts: [
    {
      scriptPayload: 'd37c4c809fe9840e7bfa77b86bd47163f6fb6c60',
      scriptType: 'p2sh'
    }
  ],
  tokens: []
}

should loosely deep-equal

[
  {
    scriptPayload: 'd37c4c809fe9840e7bfa77b86bd47163f6fb6c60',
    scriptType: 'p2sh'
  }
]
    at Context.<anonymous> (test/main.test.js:117:16)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)

      + expected - actual:  failed to generate Mocha diff

Each failure log is accessible here:
alias-server chronikWsHandler.js: initializeWebsocket returns expected websocket object for a p2pkh address.alias-server chronikWsHandler.js initializeWebsocket returns expected websocket object for a p2pkh address
alias-server chronikWsHandler.js: initializeWebsocket returns expected websocket object for a p2sh address.alias-server chronikWsHandler.js initializeWebsocket returns expected websocket object for a p2sh address
alias-server main.js: main() connects to a websocket, and runs handleAppStartup() correctly.alias-server main.js main() connects to a websocket, and runs handleAppStartup() correctly

bytesofman edited the summary of this revision. (Show Details)Wed, May 22, 23:12
bytesofman updated this revision to Diff 47911.Wed, May 22, 23:18

remove debug logs, update alias-server tests

Harbormaster completed remote builds in B29095: Diff 47911.Wed, May 22, 23:18
bytesofman updated this revision to Diff 47912.Wed, May 22, 23:19

lint

Harbormaster completed remote builds in B29096: Diff 47912.Wed, May 22, 23:23
bytesofman updated this revision to Diff 47913.Wed, May 22, 23:26

fix generateMock

bytesofman edited the summary of this revision. (Show Details)Wed, May 22, 23:27
bytesofman edited the test plan for this revision. (Show Details)
Harbormaster completed remote builds in B29097: Diff 47913.Wed, May 22, 23:30
bytesofman updated this revision to Diff 47914.Wed, May 22, 23:35

support token burn parsing

bytesofman edited the test plan for this revision. (Show Details)Wed, May 22, 23:35
Harbormaster completed remote builds in B29098: Diff 47914.Wed, May 22, 23:39
bytesofman edited the summary of this revision. (Show Details)Wed, May 22, 23:39
bytesofman updated this revision to Diff 47915.Wed, May 22, 23:55

remove dev logs and comments, do not double version bump mock-chronik-client

bytesofman retitled this revision from [ecash-herald, mock-chronik-client] Migrate to in-node chronik-client to [ecash-herald, mock-chronik-client, alias-server] Migrate ecash-herald to in-node chronik-client and add supporting mock-chronik-client methods.Wed, May 22, 23:57
Harbormaster completed remote builds in B29099: Diff 47915.Wed, May 22, 23:59
bytesofman published this revision for review.Thu, May 23, 00:03
bytesofman added inline comments.
apps/alias-server/test/chronikWsHandler.test.js
110 ↗(On Diff #47914)

this shape is changed now for in-node chronik-client

However, we do not really need to test this -- it is more testing chronik-client expected behavior than alias-server expected behavior.

see comments in diff description explaining alteration of alias-server tests in this diff.

apps/alias-server/test/main.test.js
117 ↗(On Diff #47914)

ditto

apps/ecash-herald/config.js
7 ↗(On Diff #47914)

settled on this param for amount of time to wait for an avalanche finalization msg.

If we get BLK_CONNECTED, but do not get BLK_FINALIZED within waitForFinalizationMsecs, herald will send a msg explaining this.

This should preserve ecash-herald's current ability to alert for avalanche-rejected blocks.

In practice with debug logging, the finalized msg seems to come only a few seconds after the connected msg. I have not extensively tested for a good value. 30s is definitely too long. 5s may be appropriate. 10s is not really a human-noticeable delay and provides some confidence the block is really not finalized.

apps/ecash-herald/constants/tokens.js
17 ↗(On Diff #47914)

change shape of cache to match chronik.token output, since ALP is indexed here by in-node chronik

apps/ecash-herald/index.js
18 ↗(On Diff #47914)

NNG chronik has no subscribeToBlocks method. Need to subscribe to an address and then you consequently are subscribed to blocks.

We no longer need this shim.

apps/ecash-herald/package.json
28 ↗(On Diff #47914)

not a performance critical thing. used to gauge whether or not a connected block is finalized within expected interval.

https://www.npmjs.com/package/cache-manager

standard JS dep review process...this one has fewer downloads than but was updated 23 days ago and still good download amount

incredibly, this one has more than double the downloads and hasn't been updated in 4 years: https://www.npmjs.com/package/node-cache

also twice the size

JS things

35 ↗(On Diff #47914)

this is used to test the cache / timeout for block connected -> finalized. dev dep only.

PiRK added a subscriber: PiRK.Fri, May 24, 12:54
PiRK added inline comments.
apps/ecash-herald/src/events.js
136–137 ↗(On Diff #47914)

Is there a guarantee by Chronik that the block finalized message will always come after the block connected message? I imagine there is, but I'm not certain. Otherwise it might be safer to first check if the cache is already set to BLK_FINALIZED and bail out immediately.

PiRK added inline comments.Fri, May 24, 12:58
apps/ecash-herald/src/events.js
146 ↗(On Diff #47914)

nit: you can flip the logic and return here if the status is FINALIZED, to remove an indentation level for the rest of the function.

PiRK added a comment.EditedFri, May 24, 13:04

This would not print out messages for blocks that are never finalized because another block with the same height is found within a few seconds of the first and the second block ends up being finalized less than 10s after the first one is connected. Not sure if we want to see these messages, and if we do whether this should be handled in this diff or in a follow-up

The fix for this would probably be as simple as indexing the cache by "height+hash" instead of just "height"

Fabien requested changes to this revision.Fri, May 24, 14:45
Fabien added a subscriber: Fabien.

I'm not sure about the finalization thing, being time dependent looks like a bad idea as you will always get false positives/negatives.

I suggest you don't handle finalization at all in this diff, so you get feature parity with the nng version, and add the finalization in follow up diffs.

apps/ecash-herald/config.js
10 ↗(On Diff #47915)

You're missing the only (for now) Bitcoin ABC server...

This revision now requires changes to proceed.Fri, May 24, 14:45
bytesofman updated this revision to Diff 47956.Fri, May 24, 16:37
bytesofman marked 2 inline comments as done.

add abc chronik server, improve blk connected logic

bytesofman marked an inline comment as done.Fri, May 24, 16:37
bytesofman added inline comments.
apps/ecash-herald/src/events.js
136–137 ↗(On Diff #47914)

Checked with Tobias, who says chronik follows the node here -- I'm not sure but I imagine it's impossible for a block to be finalized before it is connected.

146 ↗(On Diff #47914)

nice, this is much better.

bytesofman updated this revision to Diff 47957.Fri, May 24, 16:41

set and get cache by height and hash

bytesofman added a comment.Fri, May 24, 16:41
In D16171#367284, @PiRK wrote:

This would not print out messages for blocks that are never finalized because another block with the same height is found within a few seconds of the first and the second block ends up being finalized less than 10s after the first one is connected. Not sure if we want to see these messages, and if we do whether this should be handled in this diff or in a follow-up

The fix for this would probably be as simple as indexing the cache by "height+hash" instead of just "height"

good point, updated

Harbormaster completed remote builds in B29122: Diff 47956.Fri, May 24, 16:44
bytesofman added a comment.Fri, May 24, 16:45
In D16171#367287, @Fabien wrote:

I'm not sure about the finalization thing, being time dependent looks like a bad idea as you will always get false positives/negatives.

I suggest you don't handle finalization at all in this diff, so you get feature parity with the nng version, and add the finalization in follow up diffs.

I think the justification for keeping it in is that we would learn more about the blockchain. even with this type of failure, it still improves the herald as an always-on push-notification blockchain monitoring service.

Say we get some blocks that connect and do finalize, but it takes 12s, mb this happens every 500 blocks. Would be useful to learn that, useful to check these blocks out and see if anything interesting caused this.

Mb 1 in 3000 blocks connects and does finalize, but it takes 30s. Would also be interesting to learn this.

Since the failure mode here is bad (but investigatable) msgs, I think it's more a value add than a performance issue.

Harbormaster completed remote builds in B29123: Diff 47957.Fri, May 24, 16:48
Fabien requested changes to this revision.Mon, May 27, 07:29
In D16171#367329, @bytesofman wrote:
In D16171#367287, @Fabien wrote:

I'm not sure about the finalization thing, being time dependent looks like a bad idea as you will always get false positives/negatives.

I suggest you don't handle finalization at all in this diff, so you get feature parity with the nng version, and add the finalization in follow up diffs.

I think the justification for keeping it in is that we would learn more about the blockchain. even with this type of failure, it still improves the herald as an always-on push-notification blockchain monitoring service.

Say we get some blocks that connect and do finalize, but it takes 12s, mb this happens every 500 blocks. Would be useful to learn that, useful to check these blocks out and see if anything interesting caused this.

Mb 1 in 3000 blocks connects and does finalize, but it takes 30s. Would also be interesting to learn this.

Since the failure mode here is bad (but investigatable) msgs, I think it's more a value add than a performance issue.

I agree that having a way to know if a block is final can be interesting, but that's not my point.
This diff is doing 2 things:

  • Migrate the in-node chronik
  • Add a new feature (finalization)

The first one is hard to revert because it implies a change of server. The second one not so much. So if something is broken or undesirable in practice due to finalization you won't be able to simple revert the diff.

This is why you should split this diff in 2 parts.

This revision now requires changes to proceed.Mon, May 27, 07:29
bytesofman requested review of this revision.Mon, May 27, 18:35
In D16171#367424, @Fabien wrote:
In D16171#367329, @bytesofman wrote:
In D16171#367287, @Fabien wrote:

I'm not sure about the finalization thing, being time dependent looks like a bad idea as you will always get false positives/negatives.

I suggest you don't handle finalization at all in this diff, so you get feature parity with the nng version, and add the finalization in follow up diffs.

I think the justification for keeping it in is that we would learn more about the blockchain. even with this type of failure, it still improves the herald as an always-on push-notification blockchain monitoring service.

Say we get some blocks that connect and do finalize, but it takes 12s, mb this happens every 500 blocks. Would be useful to learn that, useful to check these blocks out and see if anything interesting caused this.

Mb 1 in 3000 blocks connects and does finalize, but it takes 30s. Would also be interesting to learn this.

Since the failure mode here is bad (but investigatable) msgs, I think it's more a value add than a performance issue.

I agree that having a way to know if a block is final can be interesting, but that's not my point.
This diff is doing 2 things:

  • Migrate the in-node chronik
  • Add a new feature (finalization)

The first one is hard to revert because it implies a change of server. The second one not so much. So if something is broken or undesirable in practice due to finalization you won't be able to simple revert the diff.

This is why you should split this diff in 2 parts.

This is true and the diff should have been built this way.

At this point, I do not think we are getting good impact from the exercise (refactoring this diff to remove finalization handling with maintained block connected alerts, with the plan being to immediately add them again). The main value to the herald of switching to in-node is the availability of finalization msgs.

Because ecash-herald is not critical infra and bugs and downtime here would not involve risk beyond temporary annoyance, I would prefer to iterate through any issues here vs revert and redo this (tedious + low impact -- but necessary + one-off) indexer migration.

To mitigate any potential issues from behavior change in this migration, I have restarted the bot on current master in the ecash herald dev tg channel, which will continue to run on NNG chronik until we are confident the prod version is sufficient to meet our needs.

Fabien accepted this revision.Tue, May 28, 08:28
Fabien added inline comments.
apps/ecash-herald/config.js
11 ↗(On Diff #47957)

put it first

apps/ecash-herald/src/events.js
50 ↗(On Diff #47957)

The hash was the useful info because you need it to investigate with a node. Since you have both the height and the hash here you can just print both

This revision is now accepted and ready to land.Tue, May 28, 08:28
Closed by commit rABC9abe1c8ba966: [ecash-herald, mock-chronik-client, alias-server] Migrate ecash-herald to in… (authored by bytesofman). · Explain WhyTue, May 28, 12:56
This revision was automatically updated to reflect the committed changes.
bytesofman marked an inline comment as done.
bytesofman added a commit: rABC9abe1c8ba966: [ecash-herald, mock-chronik-client, alias-server] Migrate ecash-herald to in….
bytesofman marked an inline comment as done.Tue, May 28, 13:09

Revision Contents
Changeset List

PathSize
apps/
alias-server/
test/
14 lines
6 lines
ecash-herald/
8 lines
constants/
10 lines
6 lines
137 lines
2 lines
scripts/
82 lines
52 lines
src/
62 lines
45 lines
119 lines
20 lines
212 lines
test/
126 lines
134 lines
200 lines
23 lines
mocks/
7478 lines
9 lines
modules/
mock-chronik-client/
5 lines
47 lines
4 lines
2 lines
test/
53 lines

Diff 47980

View Options

apps/alias-server/test/chronikWsHandler.test.js

Loading...
View Options

apps/alias-server/test/main.test.js

Loading...
View Options

apps/ecash-herald/config.js

Loading...
View Options

apps/ecash-herald/constants/tokens.js

Loading...
View Options

apps/ecash-herald/index.js

Loading...
View Options

apps/ecash-herald/package-lock.json

Loading...
View Options

apps/ecash-herald/package.json

Loading...
View Options

apps/ecash-herald/scripts/generateMock.js

Loading...
View Options

apps/ecash-herald/scripts/sendMsgByBlock.js

Loading...
View Options

apps/ecash-herald/src/chronik.js

Loading...
View Options

apps/ecash-herald/src/chronikWsHandler.js

Loading...
View Options

apps/ecash-herald/src/events.js

Loading...
View Options

apps/ecash-herald/src/main.js

Loading...
View Options

apps/ecash-herald/src/parse.js

Loading...
View Options

apps/ecash-herald/test/chronik.test.js

Loading...
View Options

apps/ecash-herald/test/chronikWsHandler.test.js

Loading...
View Options

apps/ecash-herald/test/events.test.js

Loading...
View Options

apps/ecash-herald/test/main.test.js

Loading...
View Options

apps/ecash-herald/test/mocks/block.js

Loading...
View Options

apps/ecash-herald/test/parse.test.js

Loading...
View Options

modules/mock-chronik-client/README.md

Loading...
View Options

modules/mock-chronik-client/index.js

Loading...
View Options

modules/mock-chronik-client/package-lock.json

Loading...
View Options

modules/mock-chronik-client/package.json

Loading...
View Options

modules/mock-chronik-client/test/index.test.js

Loading...