T3060
Add a function to determine avalanche 'isfinalblock' status. Add a new test script to call this function. Modify schema of secrets.js to support rpc info for full node requests.
Details
Details
- Reviewers
Fabien - Group Reviewers
Restricted Project - Commits
- rABC06b537eba3b1: [alias-server] Add RPC call for isFinalBlock
cp secrets.sample.js secrets.js
Enter your rpc call info (DM me if you need info for a test server)
npm run rpc and response good
Change scripts/testRpc.js so that const blockhash === 'foo'
npm run rpc and get the error msg from the node (blockhash must be of length 64 (not 3, for 'foo'))
Change your rpc params in secrets.js so that your url is bad
npm run rpc, returns false, error msg is logged
Diff Detail
Diff Detail
- Repository
- rABC Bitcoin ABC
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 22878 Build 45376: Build Diff alias-server-tests Build 45375: arc lint + arc unit
Event Timeline
| web/alias-server/src/rpc.js | ||
|---|---|---|
| 11 ↗ | (On Diff #39104) | indeed. copy pasted directly from bch-api, which I'm glad we are no longer using. reduced to 1000ms based on what seemed reasonable in my testing of bad URLs. |
| 25 ↗ | (On Diff #39104) | I have improved the error mgmt parsing and tested it with two different likely common errors
Even if there are some cases that remain unparsed, isFinalBlock will only return true if
if this isn't the case, then isFinalBlock will not === true, and this is what will cause the app to exit the "block found" loop before processing any alias txs. |
Comment Actions
I just noticed there is no copyright header in any of the files, that's something you need to fix in another diff
| web/alias-server/scripts/testRpc.js | ||
|---|---|---|
| 8 | you might as well make it argv[1] | |
| 10 | By the doc at the top of the file, the function really should be named after what it does and the same goes for the file. Usage or help is also welcome | |
| web/alias-server/src/rpc.js | ||
| 39 | That's not as resilient as return result === true. Let's imagine the node returns no error and "result":"maybe", you'll consider it true. | |
| 53 | You don't need that branch. Just construct the message string in the if/else and always log + return false (or log directly in the branch so you don't even need nodeErrorMsg) | |
Comment Actions
Improving code quality per diff review
| web/alias-server/scripts/testRpc.js | ||
|---|---|---|
| 8 | in this case, process.argv[2], as [0] is node, [1] is the script, and [2]` is anything following e.g. console.log(process.argv) [ '/home/<user>/.nvm/versions/node/v16.14.2/bin/node', '/home/<userr>/path/to/bitcoin-abc/web/alias-server/scripts/isFinalBlock', 'foo' ] | |
| 10 | Intent is to have one script that makes all the rpc calls used in the app. For now, it's just isFinalBlock, but might use more later. Same reason the input is hardcoded. That said, it is niftier to accept custom input from the command line, and I don't know if the app will use any more rpc commands. rpc could be deprecated here with some chronik improvements. Updated. | |
| web/alias-server/src/rpc.js | ||
| 39 | patched | |
| 53 | patched | |
Comment Actions
I just noticed there is no copyright header in any of the files, that's something you need to fix in another diff
Added to T3060
Comment Actions
It's much better now, only missing a unit test for isFinalBlock. You'll need to mock the node response here, and that will be useful to test the upper logic as well in later diffs.
| web/alias-server/src/rpc.js | ||
|---|---|---|
| 56 ↗ | (On Diff #39113) | If you return in the first branch, you don't need the else. |