fix(dashmate): replace dead mainnet tenderdash seeds; generate missing node key outside the wizard #4539
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,105 @@ | ||
| import generateTenderdashNodeKey from './generateTenderdashNodeKey.js'; | ||
| import deriveTenderdashNodeId from './deriveTenderdashNodeId.js'; | ||
|
|
||
| /** | ||
| * @param {ConfigFileJsonRepository} configFileRepository | ||
| * @return {ensureTenderdashNodeKey} | ||
| */ | ||
| export default function ensureTenderdashNodeKeyFactory(configFileRepository) { | ||
| /** | ||
| * Persist node identity values into the stored copy of the config, so a | ||
| * restart reuses the same identity instead of generating a new one. The | ||
| * config file is re-read under its lock, and a value that appeared there in | ||
| * the meantime wins over the one generated here. | ||
| * | ||
| * For a command holding the lock across its run this is an intermediate | ||
| * write: it persists the identity ahead of the command's own final save, | ||
| * without that command's other pending in-memory edits. Those still land | ||
| * with the final save; only if the command dies first does the identity | ||
| * outlive them - which is the point, since the rendered files already | ||
| * reference it. | ||
| * | ||
| * @param {Config} config | ||
| * @param {string} id | ||
| * @param {string} key | ||
| * @returns {void} | ||
| */ | ||
| function persistNodeIdentity(config, id, key) { | ||
| configFileRepository.update((configFile) => { | ||
|
Collaborator
There was a problem hiding this comment. 🔴 Blocking: Do not save the migrated config during template rendering During source: ['claude'] |
||
| // A config not stored yet (a preset being set up) is persisted by the | ||
| // command that created it once it saves the config file it holds. | ||
| if (!configFile.isConfigExists(config.getName())) { | ||
| return; | ||
| } | ||
|
|
||
| const storedConfig = configFile.getConfig(config.getName()); | ||
| const storedKey = storedConfig.get('platform.drive.tenderdash.node.key'); | ||
|
|
||
| if (storedKey === null || storedKey === key) { | ||
| storedConfig.set('platform.drive.tenderdash.node.id', id); | ||
| storedConfig.set('platform.drive.tenderdash.node.key', key); | ||
| } else { | ||
| // Another process stored a different identity first; render with | ||
| // theirs, deriving the id when it is not stored either. | ||
| config.set( | ||
| 'platform.drive.tenderdash.node.id', | ||
| storedConfig.get('platform.drive.tenderdash.node.id') ?? deriveTenderdashNodeId(storedKey), | ||
| ); | ||
| config.set('platform.drive.tenderdash.node.key', storedKey); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| /** | ||
| * Fill in a missing tenderdash node identity before service configs are | ||
| * rendered. | ||
| * | ||
| * The interactive setup wizard is the only flow that collects a node key, so | ||
| * a config assembled any other way (dashmate config create, non-interactive | ||
| * setup, enabling platform on an existing node) reaches template rendering | ||
| * with platform.drive.tenderdash.node.{id,key} still null, and node_key.json | ||
| * is written with the literal string "null" - tenderdash panics at startup. | ||
| * An existing key is never touched. | ||
| * | ||
| * @typedef {ensureTenderdashNodeKey} | ||
| * @param {Config} config | ||
| * @returns {void} | ||
| */ | ||
| function ensureTenderdashNodeKey(config) { | ||
| if (config.get('platform.enable') !== true) { | ||
| return; | ||
| } | ||
|
|
||
| // The base config is a template: a key generated for it would be cloned | ||
| // into every config created from it, and those must not share an identity. | ||
| if (config.getName() === 'base') { | ||
| return; | ||
| } | ||
|
|
||
| const existingKey = config.get('platform.drive.tenderdash.node.key'); | ||
|
|
||
| if (existingKey !== null) { | ||
| // The id is derivable, so a config carrying a key without one is | ||
| // completed rather than rejected. | ||
| if (config.get('platform.drive.tenderdash.node.id') === null) { | ||
| const id = deriveTenderdashNodeId(existingKey); | ||
|
|
||
| config.set('platform.drive.tenderdash.node.id', id); | ||
|
|
||
| persistNodeIdentity(config, id, existingKey); | ||
| } | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| const key = generateTenderdashNodeKey(); | ||
| const id = deriveTenderdashNodeId(key); | ||
|
|
||
| config.set('platform.drive.tenderdash.node.id', id); | ||
| config.set('platform.drive.tenderdash.node.key', key); | ||
|
|
||
| persistNodeIdentity(config, id, key); | ||
|
Comment on lines
+95
to
+101
Collaborator
There was a problem hiding this comment. 🟡 Suggestion: Persist generated node identities with private file permissions This path automatically creates a private Ed25519 P2P identity for nodes that did not pass through the interactive wizard, but both persistence sinks use process-umask permissions. source: ['claude'] |
||
| } | ||
|
|
||
| return ensureTenderdashNodeKey; | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,136 @@ | ||
| import ensureTenderdashNodeKeyFactory from '../../../src/tenderdash/ensureTenderdashNodeKeyFactory.js'; | ||
| import renderServiceTemplatesFactory from '../../../src/templates/renderServiceTemplatesFactory.js'; | ||
| import deriveTenderdashNodeId from '../../../src/tenderdash/deriveTenderdashNodeId.js'; | ||
| import generateTenderdashNodeKey from '../../../src/tenderdash/generateTenderdashNodeKey.js'; | ||
| import validateTenderdashNodeKey from '../../../src/listr/prompts/validators/validateTenderdashNodeKey.js'; | ||
| import Config from '../../../src/config/Config.js'; | ||
| import createDIContainer from '../../../src/createDIContainer.js'; | ||
|
|
||
| describe('ensureTenderdashNodeKeyFactory', () => { | ||
| let container; | ||
| let config; | ||
| let storedConfig; | ||
| let configFileRepository; | ||
| let ensureTenderdashNodeKey; | ||
|
|
||
| const NODE_ID_PATH = 'platform.drive.tenderdash.node.id'; | ||
| const NODE_KEY_PATH = 'platform.drive.tenderdash.node.key'; | ||
|
|
||
| beforeEach(async function beforeEach() { | ||
| container = await createDIContainer(); | ||
|
|
||
| const defaultConfigs = container.resolve('defaultConfigs'); | ||
|
|
||
| config = new Config('testnet', defaultConfigs.get('testnet').getStoredOptions()); | ||
| config.set('platform.enable', true); | ||
|
|
||
| // The stored copy the repository would read back from disk | ||
| storedConfig = new Config('testnet', config.getStoredOptions()); | ||
|
|
||
| const configFile = { | ||
| isConfigExists: this.sinon.stub().returns(true), | ||
| getConfig: this.sinon.stub().returns(storedConfig), | ||
| }; | ||
|
|
||
| configFileRepository = { | ||
| update: this.sinon.stub().callsFake((mutate) => mutate(configFile)), | ||
| }; | ||
|
|
||
| ensureTenderdashNodeKey = ensureTenderdashNodeKeyFactory(configFileRepository); | ||
| }); | ||
|
|
||
| it('should generate and persist a valid node key when the stored key is null', () => { | ||
| expect(config.get(NODE_KEY_PATH)).to.equal(null); | ||
|
|
||
| ensureTenderdashNodeKey(config); | ||
|
|
||
| const key = config.get(NODE_KEY_PATH); | ||
|
|
||
| expect(key).to.be.a('string'); | ||
| expect(validateTenderdashNodeKey(key)).to.equal(true); | ||
| expect(config.get(NODE_ID_PATH)).to.equal(deriveTenderdashNodeId(key)); | ||
|
|
||
| // Persisted into the stored copy so a restart reuses the same identity | ||
| expect(configFileRepository.update).to.have.been.calledOnce(); | ||
| expect(storedConfig.get(NODE_KEY_PATH)).to.equal(key); | ||
| expect(storedConfig.get(NODE_ID_PATH)).to.equal(config.get(NODE_ID_PATH)); | ||
| }); | ||
|
|
||
| it('should never regenerate an existing node key', () => { | ||
| const existingKey = generateTenderdashNodeKey(); | ||
| const existingId = deriveTenderdashNodeId(existingKey); | ||
|
|
||
| config.set(NODE_ID_PATH, existingId); | ||
| config.set(NODE_KEY_PATH, existingKey); | ||
|
|
||
| ensureTenderdashNodeKey(config); | ||
|
|
||
| expect(config.get(NODE_KEY_PATH)).to.equal(existingKey); | ||
| expect(config.get(NODE_ID_PATH)).to.equal(existingId); | ||
| expect(configFileRepository.update).to.have.not.been.called(); | ||
| }); | ||
|
|
||
| it('should derive and persist a missing node id from an existing key', () => { | ||
| const existingKey = generateTenderdashNodeKey(); | ||
|
|
||
| config.set(NODE_KEY_PATH, existingKey); | ||
| storedConfig.set(NODE_KEY_PATH, existingKey); | ||
|
|
||
| ensureTenderdashNodeKey(config); | ||
|
|
||
| expect(config.get(NODE_KEY_PATH)).to.equal(existingKey); | ||
| expect(config.get(NODE_ID_PATH)).to.equal(deriveTenderdashNodeId(existingKey)); | ||
| expect(storedConfig.get(NODE_ID_PATH)).to.equal(deriveTenderdashNodeId(existingKey)); | ||
| }); | ||
|
|
||
| it('should adopt an identity another process stored first', () => { | ||
| const winningKey = generateTenderdashNodeKey(); | ||
| const winningId = deriveTenderdashNodeId(winningKey); | ||
|
|
||
| storedConfig.set(NODE_ID_PATH, winningId); | ||
| storedConfig.set(NODE_KEY_PATH, winningKey); | ||
|
|
||
| ensureTenderdashNodeKey(config); | ||
|
|
||
| expect(config.get(NODE_KEY_PATH)).to.equal(winningKey); | ||
| expect(config.get(NODE_ID_PATH)).to.equal(winningId); | ||
| }); | ||
|
|
||
| it('should not touch a config with platform disabled', () => { | ||
| config.set('platform.enable', false); | ||
|
|
||
| ensureTenderdashNodeKey(config); | ||
|
|
||
| expect(config.get(NODE_KEY_PATH)).to.equal(null); | ||
| expect(configFileRepository.update).to.have.not.been.called(); | ||
| }); | ||
|
|
||
| it('should not generate a key for the base template config', () => { | ||
| const baseConfig = new Config('base', config.getStoredOptions()); | ||
|
|
||
| ensureTenderdashNodeKey(baseConfig); | ||
|
|
||
| expect(baseConfig.get(NODE_KEY_PATH)).to.equal(null); | ||
| expect(configFileRepository.update).to.have.not.been.called(); | ||
| }); | ||
|
|
||
| it('should render node_key.json with a generated key instead of "null"', () => { | ||
| // Regression: a fullnode configured outside the interactive setup wizard | ||
| // reached template rendering with a null node key, and node_key.json was | ||
| // written with the literal string "null" - tenderdash panicked at startup. | ||
| const renderTemplate = container.resolve('renderTemplate'); | ||
| const renderServiceTemplates = renderServiceTemplatesFactory( | ||
| renderTemplate, | ||
| ensureTenderdashNodeKey, | ||
| ); | ||
|
|
||
| const serviceConfigs = renderServiceTemplates(config); | ||
|
|
||
| const nodeKeyFile = JSON.parse(serviceConfigs['platform/drive/tenderdash/node_key.json']); | ||
|
|
||
| expect(nodeKeyFile.priv_key.value).to.equal(config.get(NODE_KEY_PATH)); | ||
| expect(nodeKeyFile.priv_key.value).to.not.equal('null'); | ||
| expect(nodeKeyFile.id).to.equal(config.get(NODE_ID_PATH)); | ||
| expect(validateTenderdashNodeKey(nodeKeyFile.priv_key.value)).to.equal(true); | ||
| }); | ||
| }); |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔴 Blocking: Check true set equality before replacing stored seeds
The length check followed by
every(deadSeeds.includes(...))permits duplicate entries and therefore does not establish set equality. For example, a five-entry custom list containing legacy seeds A, A, B, C, and D passes even though it omits E, so the migration replaces a list that is not equal to the stock defaults. The seed schema does not prohibit duplicates, and this replacement behavior was reproduced against the migration. Compare the unique stored addresses with every legacy default so custom lists remain untouched as promised.source: ['claude']