trjhtr

I want to get advice about functional programming using javascript promise. Please look through my code and give me a feedback how I could make code better. As I begin to use promise a few days ago, it would be great why it is better way to alter codes following feedbacks.

I implemented basic ethereum transfer codes with web3js, ganache, and nodejs. Payment is main function and it would be helpful if you could give me feedbacks about functions including it. It consists of functions, which are

1. getting account objects of sender/receiver for further use


2. checking balances of sender/receiver before transaction


3. transfering ether from sender to receiver


4. checking balances of sender/receiver after transaction

const Web3 = require('web3')

if (typeof web3 != 'undefined') 
 var web3 = new Web3(Web3.currentProvider)
 else 
 var web3 = new Web3(new Web3.providers.WebsocketProvider('ws://localhost:8545')) // NOTE : ganache-cli -p 8545 will open up port


// NOTE : input(privateKey) => output(Promise:account)
var getAccount = function (privateKey) 
 return new Promise(function (resolve, reject) 
 resolve(web3.eth.accounts.privateKeyToAccount(privateKey))
 )


// NOTE : input(address) => output(Promise:balance)
var getBalance = function (address) 
 return web3.eth.getBalance(address)
 .then(function (balance) console.log(balance);)
 .catch(function (error) throw new Error('getBalance Error'))


// NOTE : Using webjs(v1.0) and ganache, payment from one account to another
// NOTE : input(txInfo) => transferEther(sender=>receiver) => output(status)
var transferEther = function (sender, rawTx) 
 return sender.signTransaction(rawTx)
 .then(function (signedTx) 
 return web3.eth.sendSignedTransaction(signedTx.rawTransaction)
 )
 .then(function (receipt) 
 console.log(receipt)
 )
 .catch(function (error) 
 console.log(error)
 )


// Case2 : signTransaction using privateKey
var transferEtherWithPvkey = function (sender, rawTx) 
 return web3.eth.accounts.signTransaction(rawTx, sender.privateKey)
 .then(function (signedTx) 
 return web3.eth.sendSignedTransaction(signedTx.rawTransaction)
 )
 .then(function (receipt) 
 console.log(receipt)
 )
 .catch(function (error) 
 console.log(error)
 )


// Case3 : signTransaction using another entity's account
// NOTE : Relevant web3js libraries Not supported yet
// var transferEtherOfAnotherEntity = function (sender, rawTx) 
// return web3.eth.personal.unlockAccount(sender.address, '')
// .then(function (result) 
// if (result) return web3.eth.personal.signTransaction(rawTx, '')
// )
// .then(function (signedTx) 
// return web3.eth.personal.sendTransaction(signedTx.rawTransaction)
// )
// .catch(function (error) 
// console.log(error)
// )
// 

// TODO : Remove fixture
var paymentInfo = 
 pvKeys: // hardcoded private keys 
 sender: '0x32d4e4b8deae4967f6ec305683921b1d46517767ef7a3411c27bbcf24fa7b757',
 receiver: '0x90e40b307bd5ee5c7f5285aecffcf0fb223ff1cf802d913237ecaf2f962e251e'
 ,
 txInfo: 
 gasPrice: '200', // string
 gas: '210000', // string
 value: '1000', // string
 data: '' // string
 


var payment = function (paymentInfo) // TODO : Replace parameters with relevant params
 Promise.all([getAccount(paymentInfo.pvKeys.sender), getAccount(paymentInfo.pvKeys.receiver)]) // STEP 1 : get accounts of sender/receiver
 .then(function (accounts) 
 var rawTx = 
 from: accounts[0].address, // sender
 to: accounts[1].address, // receiver
 gasPrice: paymentInfo.txInfo.gasPrice,
 gas: paymentInfo.txInfo.gas,
 value: paymentInfo.txInfo.value,
 data: paymentInfo.txInfo.data
 

 Promise.all(accounts.map(account => getBalance(account.address))) // STEP 2 : check balances of sender/receiver before transaction
 .then(() => transferEther(accounts[0], rawTx)) // STEP 3 : transferEther from sender to receiver
 .then(() => accounts.map(account => getBalance(account.address))) // STEP 4 : check balances of sender/receiver after transaction
 )


payment(paymentInfo)

module.exports = web3

Node.js should already have support for await/async. You can ditch the Promise syntax in favor of a simpler, synchronous-looking syntax. For instance, payment could be like:

const payment = async (paymentInfo) 
 const accounts = await Promise.all([
 getAccount(paymentInfo.pvKeys.sender), 
 getAccount(paymentInfo.pvKeys.receiver)
 ])

 var rawTx = 
 from: accounts[0].address, // sender
 to: accounts[1].address, // receiver
 gasPrice: paymentInfo.txInfo.gasPrice,
 gas: paymentInfo.txInfo.gas,
 value: paymentInfo.txInfo.value,
 data: paymentInfo.txInfo.data
 

 await Promise.all(accounts.map(account => getBalance(account.address)))
 await transferEther(accounts[0], rawTx)

 // You're just looping through. Use forEach instead of map.
 accounts.forEach(account => getBalance(account.address))


// Async functions return a promise, and Node yells at you for unhandled
// promises. So we attach a then that appropriately handles it.
payment(paymentInfo).then(() => 
 console.log('completed')
, e => 
 console.warn(e)
)

if (typeof web3 != 'undefined') 
 var web3 = new Web3(Web3.currentProvider)
 else 
 var web3 = new Web3(new Web3.providers.WebsocketProvider('ws://localhost:8545')) // NOTE : ganache-cli -p 8545 will open up port

Each module has its own scope. There's no way web3 could have existed before this part of the code. That's unless you're introducing web3 as some Node global (which you shouldn't). Also the code is redundant. The only thing that's different is the provider. You only need to apply the conditional to select the provider, not the instantiation.

const provider = ifWeb3Exists ? Web3.currentProvider : new Web3.providers.WebsocketProvider('ws://localhost:8545')
const web3 = new Web3(provider)

// TODO : Remove fixture
var paymentInfo = 
 pvKeys: // hardcoded private keys 
 sender: '0x32d4e4b8deae4967f6ec305683921b1d46517767ef7a3411c27bbcf24fa7b757',
 receiver: '0x90e40b307bd5ee5c7f5285aecffcf0fb223ff1cf802d913237ecaf2f962e251e'
 ,
 txInfo: 
 gasPrice: '200', // string
 gas: '210000', // string
 value: '1000', // string
 data: '' // string
 

Never hardcode sensitive information in code. For instance, if this key was really important, and I just copied it to my answer, there's no way for you to remove this from my answer (unless you have editing permissions, or a moderator removes my post). Push these values off the code and into something like environment variables or arguments.

// NOTE : Using webjs(v1.0) and ganache, payment from one account to another
// NOTE : input(txInfo) => transferEther(sender=>receiver) => output(status)
var transferEther = function (sender, rawTx) 
 return sender.signTransaction(rawTx)
 .then(function (signedTx) 
 return web3.eth.sendSignedTransaction(signedTx.rawTransaction)
 )
 .then(function (receipt) 
 console.log(receipt)
 )
 .catch(function (error) 
 console.log(error)
 )

The problem with this code is that you attached a catch for logging, but did not re-throw the error. When the async calls in the function reject, the catch recovers from the error. This causes the returned promise to always resolve, never reject, and settle with an undefined value. If you're just then-ing for logging, always return the resolved value, or rethrow the rejection value.

Also, the catch is redundant. You could just stick a second callback to the last then and it should just work like catch. But then again, async/await:

const transferEther = async (sender, rawTx) => 
 const signedTx = await sender.signTransaction(rawTx)
 const receipt = await web3.eth.sendSignedTransaction(signedTx.rawTransaction)
 console.log(receipt)
 return receipt