Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(Refactor) Of method getOwnerApplicationsInstancesForProxy #1046

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
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
42 changes: 18 additions & 24 deletions src/utils/blockchainHelpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -348,34 +348,28 @@ async function getOwnerApplicationsInstancesForProxy() {
const { web3 } = web3Store
const proxiesRegistryContract = await attachToSpecificCrowdsaleContract('ProxiesRegistry')
const accounts = await web3.eth.getAccounts()
const execIDs = await proxiesRegistryContract.methods.getCrowdsalesForUser(accounts[0]).call()

const promises = []
const crowdsales = []
const proxyAddrs = await proxiesRegistryContract.methods.getCrowdsalesForUser(accounts[0]).call()
const { abi } = contractStore.MintedCappedProxy
const mintedCapped = process.env[`${REACT_PREFIX}MINTED_CAPPED_APP_NAME`].toLowerCase()
const dutchAuction = process.env[`${REACT_PREFIX}DUTCH_APP_NAME`].toLowerCase()
proxyAddrs.forEach(proxyAddr => {
let promise = new Promise(async (resolve, reject) => {
const abi = contractStore.MintedCappedProxy.abi // we can use minted caped proxy ABI for minted capped and Dutch acution
try {
const contractInstance = await attachToContract(abi, proxyAddr)
const contractAppName = await contractInstance.methods.app_name().call()
const appName = removeTrailingNUL(web3.utils.toAscii(contractAppName))
const appNameLowerCase = appName.toLowerCase()
if (appNameLowerCase.includes(mintedCapped) || appNameLowerCase.includes(dutchAuction)) {
crowdsales.push({ appName, execID: proxyAddr })
}
resolve()
} catch (error) {
logger.error(error)
resolve()
const contracts = []

for (const execID of execIDs) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

@fernandomg if I'm not wrong, in this case, smart-contracts' functions' execution will be made in a sequence, instead of parallel calls (from the current implementation). Thus, suggested implementation is slower for the number of proxy addresses greater then 1, isn't it? What are the benefits of suggested implementation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're completely right, there's no benefit from my suggestion. My main idea was to make it more readable, but there's no performance benefit from it. I'll just close it.

Thank you!

try {
const contractInstance = await attachToContract(abi, execID)
const contractAppName = await contractInstance.methods.app_name().call()
const appName = removeTrailingNUL(web3.utils.toAscii(contractAppName)).toLowerCase()

if (appName.includes(mintedCapped) || appName.includes(dutchAuction)) {
contracts.push({ appName, execID })
}
})
promises.push(promise)
})
return Promise.all(promises).then(() => {
return Promise.all(crowdsales)
})
} catch (error) {
logger.error(error)
}
}

return contracts.reverse()
}

async function getOwnerApplicationsInstances() {
Expand Down