From 1fc775958365670650812d468b60a417ffe0e1a4 Mon Sep 17 00:00:00 2001 From: jamaljsr <1356600+jamaljsr@users.noreply.github.com> Date: Thu, 12 Dec 2019 20:31:21 -0500 Subject: [PATCH] fix(bitcoind): fix LN node to bitcoin link missing after removal --- TODO.md | 8 +++++--- .../designer/lightning/actions/RemoveNode.tsx | 4 ++-- src/i18n/locales/en-US.json | 2 +- src/i18n/locales/es.json | 10 +++++++++- src/store/models/bitcoind.ts | 7 +++---- src/store/models/designer.ts | 15 +++++++++----- src/store/models/network.spec.ts | 8 ++++---- src/store/models/network.ts | 20 ++++++++++++------- src/utils/chart.ts | 17 ++++++++++++++++ 9 files changed, 64 insertions(+), 27 deletions(-) diff --git a/TODO.md b/TODO.md index b9652cc..101fa01 100644 --- a/TODO.md +++ b/TODO.md @@ -1,11 +1,13 @@ # TODO List -- refactor network model removeNode -> removeLightningNode -- update chart to reflect new backend connection +- fix deposit funds error - add new versions of LND and bitcoind - add toggle to default sidebar to hide/show older versions -- display compatibility warnings with LND + bitcoind - connect bitcoin peers after they all start up +- investigate c-lightning pending channel after opening +- drag to link LN node to new backend +- prompt for the new backend +- display compatibility warnings with LND + bitcoind Small Stuff diff --git a/src/components/designer/lightning/actions/RemoveNode.tsx b/src/components/designer/lightning/actions/RemoveNode.tsx index 80be65d..5088583 100644 --- a/src/components/designer/lightning/actions/RemoveNode.tsx +++ b/src/components/designer/lightning/actions/RemoveNode.tsx @@ -11,7 +11,7 @@ interface Props { const RemoveNode: React.FC = ({ node }) => { const { l } = usePrefixedTranslation('cmps.designer.lightning.actions.RemoveNode'); const { notify } = useStoreActions(s => s.app); - const { removeNode } = useStoreActions(s => s.network); + const { removeLightningNode } = useStoreActions(s => s.network); let modal: any; const showRemoveModal = () => { @@ -24,7 +24,7 @@ const RemoveNode: React.FC = ({ node }) => { cancelText: l('cancelBtn'), onOk: async () => { try { - await removeNode({ node }); + await removeLightningNode({ node }); notify({ message: l('success', { name }) }); } catch (error) { notify({ message: l('error'), error }); diff --git a/src/i18n/locales/en-US.json b/src/i18n/locales/en-US.json index 2cdb30c..01fdadb 100644 --- a/src/i18n/locales/en-US.json +++ b/src/i18n/locales/en-US.json @@ -14,7 +14,7 @@ "cmps.designer.bitcoind.actions.RemoveNode.title": "Remove Node From Network", "cmps.designer.bitcoind.actions.RemoveNode.btnText": "Remove", "cmps.designer.bitcoind.actions.RemoveNode.confirmTitle": "Are you sure you want to remove {{name}} from the network?", - "cmps.designer.bitcoind.actions.RemoveNode.confirmText": "Any lightning nodes using this node as a backend will be connected to another bitcoin node.", + "cmps.designer.bitcoind.actions.RemoveNode.confirmText": "Any lightning nodes using this node as a backend will be connected to the first bitcoin node.", "cmps.designer.bitcoind.actions.RemoveNode.restartText": "The network will be restarted to connect LN nodes to another backend and bitcoin nodes to other peers. This can take up to a minute to complete.", "cmps.designer.bitcoind.actions.RemoveNode.confirmBtn": "Yes", "cmps.designer.bitcoind.actions.RemoveNode.cancelBtn": "Cancel", diff --git a/src/i18n/locales/es.json b/src/i18n/locales/es.json index 28d870a..a749196 100644 --- a/src/i18n/locales/es.json +++ b/src/i18n/locales/es.json @@ -11,7 +11,15 @@ "cmps.common.OpenTerminalButton.title": "Terminal", "cmps.common.OpenTerminalButton.btn": "Lanzamiento", "cmps.common.OpenTerminalButton.info": "Ejecute los comandos '{{cmd}}' directamente en el nodo", - "cmps.designer.bitcoind.ActionsTab.notStarted": "Nodo debe iniciarse para realizar acciones en él", + "cmps.designer.bitcoind.actions.RemoveNode.title": "Eliminar nodo de la red", + "cmps.designer.bitcoind.actions.RemoveNode.btnText": "Retirar", + "cmps.designer.bitcoind.actions.RemoveNode.confirmTitle": "¿Estás seguro de que deseas eliminar {{name}} de la red?", + "cmps.designer.bitcoind.actions.RemoveNode.confirmText": "Cualquier nodo lightning que use este nodo como backend se conectará al primer nodo bitcoin.", + "cmps.designer.bitcoind.actions.RemoveNode.restartText": "La red se reiniciará para conectar los nodos LN a otro backend y los nodos bitcoin a otros pares. Esto puede tardar hasta un minuto en completarse.", + "cmps.designer.bitcoind.actions.RemoveNode.confirmBtn": "Si", + "cmps.designer.bitcoind.actions.RemoveNode.cancelBtn": "Cancelar", + "cmps.designer.bitcoind.actions.RemoveNode.success": "El nodo {{name}} se ha eliminado de la red", + "cmps.designer.bitcoind.actions.RemoveNode.error": "No se puede eliminar el nodo", "cmps.designer.bitcoind.BitcoinDetails.info": "Informacion", "cmps.designer.bitcoind.BitcoinDetails.connect": "Conectar", "cmps.designer.bitcoind.BitcoinDetails.actions": "Comportamiento", diff --git a/src/store/models/bitcoind.ts b/src/store/models/bitcoind.ts index a44187d..ed0f0d4 100644 --- a/src/store/models/bitcoind.ts +++ b/src/store/models/bitcoind.ts @@ -55,17 +55,16 @@ const bitcoindModel: BitcoindModel = { actions.setWalletinfo({ node, walletInfo }); }), mine: thunk(async (actions, { blocks, node }, { injections, getStoreState }) => { - if (blocks < 0) { - throw new Error(l('mineError')); - } + if (blocks < 0) throw new Error(l('mineError')); + await injections.bitcoindService.mine(blocks, node.ports.rpc); + // add a small delay to allow the block to propagate to all nodes await delay(500); // update info for all bitcoin nodes const network = getStoreState().network.networkById(node.networkId); await Promise.all( network.nodes.bitcoin.filter(n => n.status === Status.Started).map(actions.getInfo), ); - await actions.getInfo(node); }), }; diff --git a/src/store/models/designer.ts b/src/store/models/designer.ts index 61fe51e..6f493a1 100644 --- a/src/store/models/designer.ts +++ b/src/store/models/designer.ts @@ -105,15 +105,17 @@ const designerModel: DesignerModel = { }), syncChart: thunk( async (actions, network, { getState, getStoreState, getStoreActions }) => { - // fetch data from all of the nodes - await Promise.all( - network.nodes.lightning.map(getStoreActions().lightning.getAllInfo), - ); + if (network.status === Status.Started) { + // fetch data from all of the nodes + await Promise.all( + network.nodes.lightning.map(getStoreActions().lightning.getAllInfo), + ); + } const nodesData = getStoreState().lightning.nodes; const { allCharts } = getState(); // sync the chart with data from all of the nodes - const chart = updateChartFromNodes(allCharts[network.id], nodesData); + const chart = updateChartFromNodes(allCharts[network.id], network, nodesData); actions.setAllCharts({ ...allCharts, [network.id]: chart, @@ -269,6 +271,9 @@ const designerModel: DesignerModel = { portId: fromPortId, }, to: {}, + properties: { + type: 'link-start', + }, }; }, ), diff --git a/src/store/models/network.spec.ts b/src/store/models/network.spec.ts index a63d799..2111872 100644 --- a/src/store/models/network.spec.ts +++ b/src/store/models/network.spec.ts @@ -188,7 +188,7 @@ describe('Network model', () => { it('should remove a node from an existing network', async () => { store.getActions().designer.setActiveId(1); const node = firstNetwork().nodes.lightning[0]; - store.getActions().network.removeNode({ node }); + store.getActions().network.removeLightningNode({ node }); const { lightning } = firstNetwork().nodes; expect(lightning).toHaveLength(2); expect(lightning[0].name).toBe('bob'); @@ -197,7 +197,7 @@ describe('Network model', () => { it('should remove a c-lightning node from an existing network', async () => { store.getActions().designer.setActiveId(1); const node = firstNetwork().nodes.lightning[2]; - store.getActions().network.removeNode({ node }); + store.getActions().network.removeLightningNode({ node }); const { lightning } = firstNetwork().nodes; expect(lightning).toHaveLength(2); }); @@ -205,8 +205,8 @@ describe('Network model', () => { it('should throw an error if the network id is invalid', async () => { const node = firstNetwork().nodes.lightning[0]; node.networkId = 999; - const { removeNode } = store.getActions().network; - await expect(removeNode({ node })).rejects.toThrow( + const { removeLightningNode } = store.getActions().network; + await expect(removeLightningNode({ node })).rejects.toThrow( "Network with the id '999' was not found.", ); }); diff --git a/src/store/models/network.ts b/src/store/models/network.ts index 5c5d02c..1b6d160 100644 --- a/src/store/models/network.ts +++ b/src/store/models/network.ts @@ -61,7 +61,12 @@ export interface NetworkModel { RootModel, Promise >; - removeNode: Thunk; + removeLightningNode: Thunk< + NetworkModel, + { node: LightningNode }, + StoreInjections, + RootModel + >; removeBitcoinNode: Thunk< NetworkModel, { node: BitcoinNode }, @@ -174,7 +179,7 @@ const networkModel: NetworkModel = { await injections.dockerService.saveComposeFile(network); return node; }), - removeNode: thunk( + removeLightningNode: thunk( async (actions, { node }, { getState, injections, getStoreActions }) => { const networks = getState().networks; const network = networks.find(n => n.id === node.networkId); @@ -214,10 +219,11 @@ const networkModel: NetworkModel = { if (bitcoin.length === 1) throw new Error('Cannot remove the only bitcoin node'); const index = bitcoin.indexOf(node); - // update LN nodes to use a different backend + // update LN nodes to use a different backend. Use the first bitcoin node unless + // it is the one being removed lightning .filter(n => n.backendName === node.name) - .forEach(n => (n.backendName = bitcoin[0].name)); + .forEach(n => (n.backendName = bitcoin[index === 0 ? 1 : 0].name)); // bitcoin nodes are peer'd with the nodes immediately before and after. if the // node being removed is in between two nodes, then connect those two nodes @@ -243,8 +249,6 @@ const networkModel: NetworkModel = { // save compose file if the network is not running await dockerService.saveComposeFile(network); } - // remove the node from the chart's redux state - designer.removeNode(node.name); // update the network in the redux state and save to disk actions.setNetworks([...networks]); await actions.save(); @@ -256,8 +260,10 @@ const networkModel: NetworkModel = { await Promise.all( lightning.map(n => lightningFactory.getService(n).waitUntilOnline(n)), ); - await designer.syncChart(network); } + // remove the node from the chart's redux state + designer.removeNode(node.name); + await designer.syncChart(network); }, ), setStatus: action((state, { id, status, only, all = true, error }) => { diff --git a/src/utils/chart.ts b/src/utils/chart.ts index e58de71..dffa7f1 100644 --- a/src/utils/chart.ts +++ b/src/utils/chart.ts @@ -185,6 +185,7 @@ const updateLinksAndPorts = ( export const updateChartFromNodes = ( chart: IChart, + network: Network, nodesData: LightningNodeMapping, ): IChart => { // create a mapping of node name to pubkey for lookups @@ -217,6 +218,22 @@ export const updateChartFromNodes = ( } }); + // ensure all lightning -> bitcoin backend links exist. one may have + // been deleted if a bitcoin node was removed + network.nodes.lightning.forEach(ln => { + const id = `${ln.name}-backend`; + if (!links[id]) { + links[id] = { + id: `${ln.name}-backend`, + from: { nodeId: ln.name, portId: 'backend' }, + to: { nodeId: ln.backendName, portId: 'backend' }, + properties: { + type: 'backend', + }, + }; + } + }); + // remove links for channels that no longer exist Object.keys(links).forEach(linkId => { // don't remove links for existing channels