stakwork / sphinx-relay

Node.js wrapper for communication between sphinx client and lightning node.
MIT License
248 stars 71 forks source link

Code convention issue #85

Open andreykutkov opened 4 years ago

andreykutkov commented 4 years ago

The codebase seems to have some points to improve code convention-wise. Two major improvements that can made are:

Example call stack:

`src/network/receive.ts`:201 initGrpcSubscriptions() <<< note B
  `src/grpc/index.ts`:15 subscribeInvoices(parseKeysendInvoice) <<< note A
    `src/utils/lightning.ts`:46 loadLightning()
       (node_module grpc).load("rpc.proto")
    [success] `src/network/receive.ts`:248 parseKeysendInvoice <<< note A
    [failure] `src/gprc/index.ts`:120 reconnectToLND()
        `src/network/receive.ts`:201 initGrpcSubscriptions() <<< note B

note A: Callback style here. Can be replaced with Promise. note B: Circular dependency. *** The code lines might be different due to frequent code update. But left there for reference assuming the difference might be not so huge.

Evanfeenstra commented 4 years ago

Thanks for your comments.

The parseKeysendInvoice callback is called many times, as invoices payments are received from LND over the GRPC subscription. How would you propose handling that with a promise?

andreykutkov commented 4 years ago

As there's only one party listening for subscribeInvoices, current callback style will work fine. If there're more than one listeners, other coding paradigms can be considered.

Regarding node B: Another drawback of circular dependency is that the call stack can get deepen resulting in possible failure of system on constant failures. So I suggest the following style. The pseudocode is:

// src/network/receive.ts
export async function initGrpcSubscriptions() {
    await getInfo() // Just leaving this will rethrow error.
    let error = InitialError();
    // The loop that retries subscribeInvoices
    while (error) {
        try {
            await lndService.subscribeInvoices(parseKeysendInvoice)
        }
        catch (caught) {
            if (error is CriticalError) {
                throw caught
            }
            else {
                error = caught;
            }
        }
    }
}

// src/grpc/index.ts
export function subscribeInvoices(parseKeysendInvoice) {
    // On success:
    parseKeysendInvoice(response)
    resolve(status);
    // On failure that requires retry
    reject(SomeError());
    // On critical failure that doesn't require retry
    rejects(CriticalError());
}