Skip to content
This repository has been archived by the owner on Jan 14, 2020. It is now read-only.

Sparrow dom/tranaction hash #546

Merged
merged 12 commits into from
Sep 27, 2018
4 changes: 2 additions & 2 deletions src/contractInterface/users/resolver.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ class UsersResolver {
this.currentAdapter = this.adapters[this.currentVersion]
}

async set({ profile, attestations = [] }) {
return this.currentAdapter.set({ profile, attestations })
async set({ profile, attestations = [], options = {} }) {
return this.currentAdapter.set({ profile, attestations, options })
}

async get(address) {
Expand Down
11 changes: 6 additions & 5 deletions src/contractInterface/users/v00_adapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,13 @@ class V00_UsersAdapter {
this.contractName = 'V00_UserRegistry'
}

async set({ profile, attestations = [] }) {
async set({ profile, attestations = [], options = {}}) {
if (profile) {
const selfAttestation = await this.profileAttestation(profile)
attestations.push(selfAttestation)
}
const newAttestations = await this.newAttestations(attestations)
return await this.addAttestations(newAttestations)
return await this.addAttestations(newAttestations, options)
}

async get(address) {
Expand Down Expand Up @@ -91,7 +91,7 @@ class V00_UsersAdapter {
})
}

async addAttestations(attestations) {
async addAttestations(attestations, options) {
const account = await this.contractService.currentAccount()
const userRegistry = await this.contractService.deployed(
this.contractService.contracts[this.contractName]
Expand Down Expand Up @@ -123,7 +123,7 @@ class V00_UsersAdapter {
'ClaimHolderRegistered',
'addClaims',
[topics, issuers, sigs, data, dataOffsets],
{ from: account, gas: 400000, contractAddress: identityAddress }
{ ...options, from: account, gas: 400000, contractAddress: identityAddress }
)
} else {
// create identity with presigned claims
Expand All @@ -138,7 +138,8 @@ class V00_UsersAdapter {
data,
dataOffsets
],
{ from: account, gas }
{ from: account, gas },
options
)
}
} else if (!identityAddress) {
Expand Down
4 changes: 2 additions & 2 deletions src/resources/users.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ class Users {
this.resolver = new UsersResolver({ contractService, ipfsService })
}

async set({ profile, attestations = [] }) {
return this.resolver.set({ profile, attestations })
async set({ profile, attestations = [], options = {}}) {
Copy link

Choose a reason for hiding this comment

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

nit: can we document as a comment what options are available ?

return this.resolver.set({ profile, attestations, options })
}

async get(address) {
Expand Down
42 changes: 40 additions & 2 deletions src/services/contract-service.js
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ class ContractService {
return withLibraryAddresses
}

async deploy(contract, args, options) {
async deploy(contract, args, options, { confirmationCallback, transactionHashCallback } = {} ) {
const bytecode = await this.getBytecode(contract)
const deployed = await this.deployed(contract)
const txReceipt = await new Promise((resolve, reject) => {
Expand All @@ -205,16 +205,53 @@ class ContractService {
.on('receipt', receipt => {
resolve(receipt)
})
//.on('confirmation', confirmationCallback)
//.on('transactionHash', transactionHashCallback)
// Workaround for "confirmationCallback" not being triggered with web3 version:1.0.0-beta.34
.on('transactionHash', (hash) => {
if (transactionHashCallback)
transactionHashCallback(hash)
if (confirmationCallback)
this.checkForDeploymentCompletion(hash, confirmationCallback)
})
.on('error', err => reject(err))
})
return txReceipt
}

/* confirmation callback does not get triggered in current version of web3 version:1.0.0-beta.34
* so this function perpetually (until 5 confirmations) checks for presence of deployed contract.
*
* This could also be a problem in Ethereum node: https://github.com/ethereum/web3.js/issues/1255
*/
async checkForDeploymentCompletion(hash, confirmationCallback) {
const transactionInfo = await this.web3.eth.getTransaction(hash)

// transaction not mined
if (transactionInfo.blockNumber === null){
setTimeout(() => {
this.checkForDeploymentCompletion(hash, confirmationCallback)
}, 1500)
} else {
const currentBlockNumber = await this.web3.eth.getBlockNumber()
const confirmations = currentBlockNumber - transactionInfo.blockNumber
confirmationCallback(confirmations, {
transactionHash: transactionInfo.hash
})
// do checks untill 5 block confirmations
Copy link

Choose a reason for hiding this comment

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

typo: "until"
nit: let's define 5 as a constant. Like NUM_CONFIRMATIONS

if (confirmations < 5) {
setTimeout(() => {
this.checkForDeploymentCompletion(hash, confirmationCallback)
}, 1500)
}
}
}

async call(
contractName,
functionName,
args = [],
{ contractAddress, from, gas, value, confirmationCallback, additionalGas = 0 } = {}
{ contractAddress, from, gas, value, confirmationCallback, transactionHashCallback, additionalGas = 0 } = {}
) {
const contractDefinition = this.contracts[contractName]
if (typeof contractDefinition === 'undefined') {
Expand All @@ -239,6 +276,7 @@ class ContractService {
.send(opts)
.on('receipt', resolve)
.on('confirmation', confirmationCallback)
Copy link

@franckc franckc Sep 27, 2018

Choose a reason for hiding this comment

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

Won't we have same issue here mentioned in the deploy method that confirmationCallback does not get triggered ? Are we relying on this callback anywhere in our DApp or origin-js codebase ? If yes let's fix it (ok to do in separate PR), if no could we at least add a comment to warn caller that this is currently not working ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Interestingly when doing an Ethereum transaction confirmation callback is working, but when deploying a contract it is not. That is why the workaround is implemented only in the deploy function a few lines above.

Copy link

Choose a reason for hiding this comment

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

Got it. Thanks for being thorough.

.on('transactionHash', transactionHashCallback)
.on('error', reject)
})
const block = await this.web3.eth.getBlock(transactionReceipt.blockNumber)
Expand Down