Page MenuHomePhabricator

[faucet] Add a faucet application
ClosedPublic

Authored by Fabien on Oct 8 2024, 14:18.

Details

Reviewers
bytesofman
Group Reviewers
Restricted Project
Commits
rABC3d313ee39669: [faucet] Add a faucet application
Summary

This is a very simple faucet that will be used to send testnet coins. It is so simple that it can be used as an example on how to use our libraries. Also I don't think unit tests would make sense for this kind of application.

Test Plan

Edit the configuration file then:

npm install
npm start

Try it out via curl requests like so:

curl http://127.0.0.1:18300/balance
curl http://127.0.0.1:18300/claim/<your address>

The logs are verbose so you can easily check what is happening.

Diff Detail

Repository
rABC Bitcoin ABC
Branch
faucet
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 30516
Build 60549: Build Diff
Build 60548: arc lint + arc unit

Event Timeline

Fabien requested review of this revision.Oct 8 2024, 14:18
Fabien planned changes to this revision.Oct 8 2024, 14:19
Fabien edited the test plan for this revision. (Show Details)

Fix the rate limit applying to the balance endpoint due to route conflict, and fix the npm start script

$ ts-node index.ts fails with

/home/joey/.nvm/versions/node/v20.11.0/lib/node_modules/ts-node/src/index.ts:859
    return new TSError(diagnosticText, diagnosticCodes, diagnostics);
           ^
TSError: ⨯ Unable to compile TypeScript:
index.ts:18:53 - error TS7016: Could not find a declaration file for module 'express'. '/home/joey/github/abc/bitcoin-abc/apps/faucet/node_modules/express/index.js' implicitly has an 'any' type.
  Try `npm i --save-dev @types/express` if it exists or add a new declaration (.d.ts) file containing `declare module 'express';`

18 import express, { Express, Request, Response } from 'express';

prob still works on building, but might as well add the types as dev dependencies

To get this app to work wtih ts-node, I had to run

npm i -D @types/express@4
npm i -D @types/cors

Beyond that -- should also install typescript and ts-node as dev-dependencies, so it is clear what version will work with compilation. For my testing, the latest versions worked, i.e.

npm i -D typescript ts-node
apps/faucet/index.ts
118 ↗(On Diff #49977)

helmet is good to use but it looks like this is not being used here?

did you mean https://www.npmjs.com/package/cors?

197 ↗(On Diff #49977)
203 ↗(On Diff #49977)

lol have not seen the "just use all the utxos" approach before, but makes sense in a faucet context. mb worth a comment explaining but no blocker.

233 ↗(On Diff #49977)

false is the default param here, so could omit it.

255–259 ↗(On Diff #49977)

here's how this appears in the logs

Successfully sent 10000 tXEC to ectest:qrg8vx3ry86dsrjzs8tck63mmscgsc3xyssryr9t2c: [object Object]

The problem is that the response from chronik is not txid directly, as expected on line 231 here; txid is a key in the object returned by chronik.broadcast...

so -- prob change txid on line 231 to response, then txid here to response.txid

295 ↗(On Diff #49977)

may not need to return here

I return in token server bc I use the app for other things.

This is a super simple app, but you may still want to model that approach -- if only to make sure processes are cleaned up properly.

e.g. check token-server's index.js for

// Gracefully shut down on app termination
        process.on('SIGTERM', () => {
            // kill <pid> from terminal
            server.close();
            console.log('token-server shut down by SIGTERM');
            // Shut down the telegram bot
            telegramBot.stopPolling();
            process.exit(0);
        });

        process.on('SIGINT', () => {
            // ctrl + c in nodejs
            server.close();
            console.log('token-server shut down by ctrl+c');
            // Shut down the telegram bot
            telegramBot.stopPolling();
            process.exit(0);
        });
bytesofman requested changes to this revision.Oct 8 2024, 17:12

I don't think unit tests would make sense for this kind of application.

Provided we never add more features, imo it's ok.

apps/faucet/index.ts
264 ↗(On Diff #49977)

same issue as above, this looks like

{"address":"ectest:qrg8vx3ry86dsrjzs8tck63mmscgsc3xyssryr9t2c","amount":1000000,"txid":{"txid":"fb48e9e63d95f2f5c578a1ad4da5619611517f653f82c92ce80fc6782c1bcb75"}}

note the double-nested txid key

This revision now requires changes to proceed.Oct 8 2024, 17:12
apps/faucet/index.ts
295 ↗(On Diff #49977)

tho, the way this app is set up, ctrl+c does kill the process for me. so, mb not necessary.

apps/faucet/index.ts
118 ↗(On Diff #49977)

I removed it because it's not needed here, but forgot the comment

203 ↗(On Diff #49977)

lol have not seen the "just use all the utxos" approach before, but makes sense in a faucet context. mb worth a comment explaining but no blocker.

I looked at how it's done in the token server and this makes more sense for a simple faucet

233 ↗(On Diff #49977)

Indeed, and it should be true XD there is no token involved here

255–259 ↗(On Diff #49977)

Good catch

295 ↗(On Diff #49977)

That was also my idea, there is nothing to clean up here

bytesofman added inline comments.
apps/faucet/index.ts
232 ↗(On Diff #49980)

this does mean that, if anyone sends testnet tokens to the faucet, they will be burned

prob ok

This revision is now accepted and ready to land.Oct 8 2024, 18:35
apps/faucet/index.ts
232 ↗(On Diff #49980)

yes it's something we want, otherwise you can DoS the faucet by sending some tokens to the wallet address

This revision was automatically updated to reflect the committed changes.