Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Harden use of geth #15029

Merged
merged 1 commit into from
Aug 15, 2018
Merged

Harden use of geth #15029

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
126 changes: 84 additions & 42 deletions app/ethWallet-geth.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const pidPath = isWindows ? '\\\\.\\pipe\\geth.pid' : path.join(gethDataDir, 'ge
const gethProcessPath = path.join(getExtensionsPath('bin'), gethProcessKey)

const configurePeers = async (dataDir) => {
const staticNodePath = path.join(dataDir, 'geth', 'static-nodes.json')
try {
const discoveryDomain = `_enode._tcp.${envNet}.${envSubDomain}.brave.com`
let newNodes = await dns.resolveSrv(discoveryDomain)
Expand All @@ -45,9 +46,10 @@ const configurePeers = async (dataDir) => {

const enodes = newNodes.map(({name, port}, i) => `enode://${newNodesPublicKeys[i]}@${newNodesIps[i]}:${port}`)

await fs.writeFile(path.join(dataDir, 'geth', 'static-nodes.json'), JSON.stringify(enodes))
} catch (e) {
console.error('Failed to configure static nodes peers ' + e.message)
await fs.writeFile(staticNodePath, JSON.stringify(enodes))
} catch (ex) {
console.error('unable to configure ' + staticNodePath + ': ' + ex.message)
throw ex
}
}

Expand Down Expand Up @@ -104,29 +106,34 @@ const spawnGeth = async () => {
}

const gethOptions = {
stdio: process.env.GETH_LOG ? 'inherit' : 'ignore'
stdio: ((envNet === 'ropsten') || process.env.GETH_LOG) ? 'inherit' : 'ignore'
Copy link
Contributor

@ryanml ryanml Aug 15, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this included intentionally? i think that would should let GETH_LOG be the sole deciding indicator for logging, lest there is value in always having debug output on ropsten

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes. it's intentional: if someone is on the test network, or someone turns on GETH_LOG, then we get logs, otherwise not. in other words, for production use, no logs by default; for testnet use, logs by default.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

++

}

ensureGethDataDir()

// If the process from the previous browswer session still lingers, it should be killed
if (await fs.pathExists(pidPath)) {
try {
const pid = await fs.readFile(pidPath)
cleanupGeth(pid)
} catch (ex) {
console.error('Could not read from geth.pid')
console.error('unable to read ' + pidPath + ': ' + ex.message)
}
}

geth = spawn(gethProcessPath, gethArgs, gethOptions)
ensureGethDataDir()

try {
geth = spawn(gethProcessPath, gethArgs, gethOptions)

geth.on('exit', handleGethStop.bind(null, 'exit'))
geth.on('close', handleGethStop.bind(null, 'close'))
geth.on('exit', handleGethStop.bind(null, 'exit'))
geth.on('close', handleGethStop.bind(null, 'close'))

await writeGethPid(geth.pid)
await writeGethPid(geth.pid)

console.warn('GETH: spawned')
console.warn('GETH: spawned')
} catch (ex) {
console.error('unable to spawn ' + gethProcessPath + ': ' + ex.message)
cleanupGeth(geth && geth.pid)
}
}

const ensureGethDataDir = () => {
Expand Down Expand Up @@ -157,44 +164,64 @@ const handleGethStop = (event, code, signal) => {

const writeGethPid = async (pid) => {
if (!pid) {
return
throw new Error('no pid returned by spawn')
}

gethProcessId = pid

try {
await fs.writeFile(pidPath, gethProcessId)
} catch (ex) {
console.error('Could not write geth.pid')
console.error('unable to write ' + pidPath + ': ' + ex.message)
throw ex
}
}

const cleanupGethAndExit = (exitCode) => {
cleanupGeth()
process.exit(exitCode || 2)
}

const cleanupGeth = (processId) => {
processId = processId || gethProcessId

if (processId) {
// Set geth to null to remove bound listeners
// Otherwise, geth will attempt to restart itself
// when killed.
geth = null
if (!processId) return console.warn('GET: nothing to cleanup')

// Set geth to null to remove bound listeners
// Otherwise, geth will attempt to restart itself
// when killed.
geth = null

// Kill process
// Kill process
try {
process.kill(processId)
} catch (ex) {
console.error('unable to kill ' + processId + ': ' + ex.message)
}
try {
process.kill(processId, 0)
console.log('GETH: unable to kill ' + processId)
} catch (ex) {
if (ex.code === 'ESRCH') {
console.log('GETH: process killed')
} else {
console.error('unable to kill ' + processId + ': ' + ex.message)
}
}

// Remove in memory process id
gethProcessId = null

// Remove in memory process id
gethProcessId = null

// Named pipes on Windows will get deleted
// automatically once no processes are using them.
if (!isWindows) {
try {
fs.unlinkSync(pidPath)
} catch (ex) {
console.error('Could not delete geth.pid')
}
// Named pipes on Windows will get deleted
// automatically once no processes are using them.
if (!isWindows) {
try {
fs.unlinkSync(pidPath)
} catch (ex) {
console.error('unable to delete ' + pidPath + ': ' + ex.message)
}
console.warn('GETH: cleanup done')
}
console.warn('GETH: cleanup done')
}

// Attempts to restart geth up to 3 times
Expand All @@ -216,16 +243,19 @@ const restartGeth = async (tries = 3) => {

// Geth should be killed on normal process, exit, SIGINT,
// and application crashing exceptions.
process.on('exit', () => {
cleanupGeth(gethProcessId)
})
process.on('SIGINT', () => {
cleanupGeth(gethProcessId)
process.exit(2)
})
process.on('exit', cleanupGeth)
process.on('SIGHUP', cleanupGethAndExit)
process.on('SIGTERM', cleanupGethAndExit)
process.on('SIGINT', cleanupGethAndExit)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for these ones ++


ipcMain.on('eth-wallet-create-wallet', (e, pwd) => {
const client = net.createConnection(ipcPath)
let client

try {
client = net.createConnection(ipcPath)
} catch (ex) {
return console.error('unable to connect to ' + ipcPath + ' (1): ' + ex.message)
}

client.on('connect', () => {
client.write(JSON.stringify({ 'method': 'personal_newAccount', 'params': [pwd], 'id': 1, 'jsonrpc': '2.0' }))
Expand All @@ -241,7 +271,13 @@ ipcMain.on('eth-wallet-create-wallet', (e, pwd) => {
})

ipcMain.on('eth-wallet-wallets', (e, data) => {
const client = net.createConnection(ipcPath)
let client

try {
client = net.createConnection(ipcPath)
} catch (ex) {
console.error('unable to connect to ' + ipcPath + ' (2): ' + ex.message)
}

client.on('connect', () => {
client.write(JSON.stringify({ 'method': 'db_putString', 'params': ['braveEthWallet', 'wallets', data], 'id': 1, 'jsonrpc': '2.0' }))
Expand All @@ -253,7 +289,13 @@ ipcMain.on('eth-wallet-wallets', (e, data) => {
})

ipcMain.on('eth-wallet-unlock-account', (e, address, pw) => {
const client = net.createConnection(ipcPath)
let client

try {
client = net.createConnection(ipcPath)
} catch (ex) {
console.error('unable to connect to ' + ipcPath + ' (3): ' + ex.message)
}

client.on('connect', () => {
client.write(JSON.stringify({ 'method': 'personal_unlockAccount', 'params': [address, pw], 'id': 1, 'jsonrpc': '2.0' }))
Expand Down