From 3fd5f58ee5c1529fda33293a90fb8054ace6d1b1 Mon Sep 17 00:00:00 2001 From: Domen Grabec Date: Tue, 25 Sep 2018 16:13:45 +0200 Subject: [PATCH 1/9] add transaction hash event to contract call --- src/contractInterface/users/resolver.js | 4 ++-- src/contractInterface/users/v00_adapter.js | 8 ++++---- src/resources/users.js | 4 ++-- src/services/contract-service.js | 3 ++- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/contractInterface/users/resolver.js b/src/contractInterface/users/resolver.js index 0a5058e6..f8bc4d28 100644 --- a/src/contractInterface/users/resolver.js +++ b/src/contractInterface/users/resolver.js @@ -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) { diff --git a/src/contractInterface/users/v00_adapter.js b/src/contractInterface/users/v00_adapter.js index 6238947e..242015e3 100644 --- a/src/contractInterface/users/v00_adapter.js +++ b/src/contractInterface/users/v00_adapter.js @@ -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) { @@ -91,7 +91,7 @@ class V00_UsersAdapter { }) } - async addAttestations(attestations) { + async addAttestations(attestations, { transactionHashCallback } = {}) { const account = await this.contractService.currentAccount() const userRegistry = await this.contractService.deployed( this.contractService.contracts[this.contractName] @@ -123,7 +123,7 @@ class V00_UsersAdapter { 'ClaimHolderRegistered', 'addClaims', [topics, issuers, sigs, data, dataOffsets], - { from: account, gas: 400000, contractAddress: identityAddress } + { from: account, gas: 400000, contractAddress: identityAddress, transactionHashCallback } ) } else { // create identity with presigned claims diff --git a/src/resources/users.js b/src/resources/users.js index cae8ac4b..704ee56a 100644 --- a/src/resources/users.js +++ b/src/resources/users.js @@ -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 = {} }) { + return this.resolver.set({ profile, attestations, options }) } async get(address) { diff --git a/src/services/contract-service.js b/src/services/contract-service.js index f2ff3524..ba991a0b 100644 --- a/src/services/contract-service.js +++ b/src/services/contract-service.js @@ -214,7 +214,7 @@ class ContractService { 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') { @@ -239,6 +239,7 @@ class ContractService { .send(opts) .on('receipt', resolve) .on('confirmation', confirmationCallback) + .on('transactionHash', transactionHashCallback) .on('error', reject) }) const block = await this.web3.eth.getBlock(transactionReceipt.blockNumber) From 95e164b115943a589b3522f237917e2b5a23daec Mon Sep 17 00:00:00 2001 From: Domen Grabec Date: Tue, 25 Sep 2018 16:48:59 +0200 Subject: [PATCH 2/9] simpify api --- src/contractInterface/users/resolver.js | 4 ++-- src/contractInterface/users/v00_adapter.js | 6 +++--- src/resources/users.js | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/contractInterface/users/resolver.js b/src/contractInterface/users/resolver.js index f8bc4d28..a5fa9d50 100644 --- a/src/contractInterface/users/resolver.js +++ b/src/contractInterface/users/resolver.js @@ -11,8 +11,8 @@ class UsersResolver { this.currentAdapter = this.adapters[this.currentVersion] } - async set({ profile, attestations = [], options = {} }) { - return this.currentAdapter.set({ profile, attestations, options }) + async set({ profile, transactionHashCallback, attestations = [] }) { + return this.currentAdapter.set({ profile, transactionHashCallback, attestations }) } async get(address) { diff --git a/src/contractInterface/users/v00_adapter.js b/src/contractInterface/users/v00_adapter.js index 242015e3..0eb32bf9 100644 --- a/src/contractInterface/users/v00_adapter.js +++ b/src/contractInterface/users/v00_adapter.js @@ -20,13 +20,13 @@ class V00_UsersAdapter { this.contractName = 'V00_UserRegistry' } - async set({ profile, attestations = [], options = {} }) { + async set({ profile, transactionHashCallback, attestations = []}) { if (profile) { const selfAttestation = await this.profileAttestation(profile) attestations.push(selfAttestation) } const newAttestations = await this.newAttestations(attestations) - return await this.addAttestations(newAttestations, options) + return await this.addAttestations(newAttestations, transactionHashCallback) } async get(address) { @@ -91,7 +91,7 @@ class V00_UsersAdapter { }) } - async addAttestations(attestations, { transactionHashCallback } = {}) { + async addAttestations(attestations, transactionHashCallback) { const account = await this.contractService.currentAccount() const userRegistry = await this.contractService.deployed( this.contractService.contracts[this.contractName] diff --git a/src/resources/users.js b/src/resources/users.js index 704ee56a..e6ccd004 100644 --- a/src/resources/users.js +++ b/src/resources/users.js @@ -5,8 +5,8 @@ class Users { this.resolver = new UsersResolver({ contractService, ipfsService }) } - async set({ profile, attestations = [], options = {} }) { - return this.resolver.set({ profile, attestations, options }) + async set({ profile, transactionHashCallback, attestations = []}) { + return this.resolver.set({ profile, transactionHashCallback, attestations }) } async get(address) { From f3610b2af997de0aa309fabd71c192e20e55bf7e Mon Sep 17 00:00:00 2001 From: Domen Grabec Date: Wed, 26 Sep 2018 21:08:37 +0200 Subject: [PATCH 3/9] add transactionHash and confirmationCallback to deploy function as well --- src/contractInterface/users/resolver.js | 4 +-- src/contractInterface/users/v00_adapter.js | 11 +++---- src/resources/users.js | 4 +-- src/services/contract-service.js | 35 +++++++++++++++++++++- 4 files changed, 44 insertions(+), 10 deletions(-) diff --git a/src/contractInterface/users/resolver.js b/src/contractInterface/users/resolver.js index a5fa9d50..ee1f4b1d 100644 --- a/src/contractInterface/users/resolver.js +++ b/src/contractInterface/users/resolver.js @@ -11,8 +11,8 @@ class UsersResolver { this.currentAdapter = this.adapters[this.currentVersion] } - async set({ profile, transactionHashCallback, attestations = [] }) { - return this.currentAdapter.set({ profile, transactionHashCallback, attestations }) + async set({ profile, transactionHashCallback, attestations = [], options = {} }) { + return this.currentAdapter.set({ profile, attestations, options }) } async get(address) { diff --git a/src/contractInterface/users/v00_adapter.js b/src/contractInterface/users/v00_adapter.js index 0eb32bf9..6aa7948b 100644 --- a/src/contractInterface/users/v00_adapter.js +++ b/src/contractInterface/users/v00_adapter.js @@ -20,13 +20,13 @@ class V00_UsersAdapter { this.contractName = 'V00_UserRegistry' } - async set({ profile, transactionHashCallback, attestations = []}) { + async set({ profile, transactionHashCallback, attestations = [], options = {}}) { if (profile) { const selfAttestation = await this.profileAttestation(profile) attestations.push(selfAttestation) } const newAttestations = await this.newAttestations(attestations) - return await this.addAttestations(newAttestations, transactionHashCallback) + return await this.addAttestations(newAttestations, options) } async get(address) { @@ -91,7 +91,7 @@ class V00_UsersAdapter { }) } - async addAttestations(attestations, transactionHashCallback) { + async addAttestations(attestations, options) { const account = await this.contractService.currentAccount() const userRegistry = await this.contractService.deployed( this.contractService.contracts[this.contractName] @@ -123,7 +123,7 @@ class V00_UsersAdapter { 'ClaimHolderRegistered', 'addClaims', [topics, issuers, sigs, data, dataOffsets], - { from: account, gas: 400000, contractAddress: identityAddress, transactionHashCallback } + { ...options, from: account, gas: 400000, contractAddress: identityAddress } ) } else { // create identity with presigned claims @@ -138,7 +138,8 @@ class V00_UsersAdapter { data, dataOffsets ], - { from: account, gas } + { from: account, gas }, + options ) } } else if (!identityAddress) { diff --git a/src/resources/users.js b/src/resources/users.js index e6ccd004..af11f5bd 100644 --- a/src/resources/users.js +++ b/src/resources/users.js @@ -5,8 +5,8 @@ class Users { this.resolver = new UsersResolver({ contractService, ipfsService }) } - async set({ profile, transactionHashCallback, attestations = []}) { - return this.resolver.set({ profile, transactionHashCallback, attestations }) + async set({ profile, attestations = [], options = {}}) { + return this.resolver.set({ profile, attestations, options }) } async get(address) { diff --git a/src/services/contract-service.js b/src/services/contract-service.js index ba991a0b..323b2fc5 100644 --- a/src/services/contract-service.js +++ b/src/services/contract-service.js @@ -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) => { @@ -205,11 +205,44 @@ 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) => { + transactionHashCallback(hash) + 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. + */ + 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 + if (confirmations < 5) { + setTimeout(() => { + this.checkForDeploymentCompletion(hash, confirmationCallback) + }, 1500) + } + } + } + async call( contractName, functionName, From 6e1cdc45ba06dcdf6e894d8c4d31e138cf0169ee Mon Sep 17 00:00:00 2001 From: Domen Grabec Date: Wed, 26 Sep 2018 21:20:57 +0200 Subject: [PATCH 4/9] handle null calllbacks --- src/services/contract-service.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/services/contract-service.js b/src/services/contract-service.js index 323b2fc5..6d13e660 100644 --- a/src/services/contract-service.js +++ b/src/services/contract-service.js @@ -209,8 +209,10 @@ class ContractService { //.on('transactionHash', transactionHashCallback) // Workaround for "confirmationCallback" not being triggered with web3 version:1.0.0-beta.34 .on('transactionHash', (hash) => { - transactionHashCallback(hash) - this.checkForDeploymentCompletion(hash, confirmationCallback) + if (transactionHashCallback) + transactionHashCallback(hash) + if (confirmationCallback) + this.checkForDeploymentCompletion(hash, confirmationCallback) }) .on('error', err => reject(err)) }) From 0bfcb1ccca3b29be55465cfe040dc21c42d9ba5d Mon Sep 17 00:00:00 2001 From: Domen Grabec Date: Wed, 26 Sep 2018 22:11:00 +0200 Subject: [PATCH 5/9] add possible probleme explanation --- src/services/contract-service.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/services/contract-service.js b/src/services/contract-service.js index 6d13e660..ed1891e1 100644 --- a/src/services/contract-service.js +++ b/src/services/contract-service.js @@ -221,6 +221,8 @@ class ContractService { /* 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) From 5f14c3a35c0ef67505c3fad1764235669b33ece4 Mon Sep 17 00:00:00 2001 From: Domen Grabec Date: Wed, 26 Sep 2018 22:19:03 +0200 Subject: [PATCH 6/9] remove unneeded options --- src/contractInterface/users/resolver.js | 2 +- src/contractInterface/users/v00_adapter.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/contractInterface/users/resolver.js b/src/contractInterface/users/resolver.js index ee1f4b1d..f8bc4d28 100644 --- a/src/contractInterface/users/resolver.js +++ b/src/contractInterface/users/resolver.js @@ -11,7 +11,7 @@ class UsersResolver { this.currentAdapter = this.adapters[this.currentVersion] } - async set({ profile, transactionHashCallback, attestations = [], options = {} }) { + async set({ profile, attestations = [], options = {} }) { return this.currentAdapter.set({ profile, attestations, options }) } diff --git a/src/contractInterface/users/v00_adapter.js b/src/contractInterface/users/v00_adapter.js index 6aa7948b..a7425ae8 100644 --- a/src/contractInterface/users/v00_adapter.js +++ b/src/contractInterface/users/v00_adapter.js @@ -20,7 +20,7 @@ class V00_UsersAdapter { this.contractName = 'V00_UserRegistry' } - async set({ profile, transactionHashCallback, attestations = [], options = {}}) { + async set({ profile, attestations = [], options = {}}) { if (profile) { const selfAttestation = await this.profileAttestation(profile) attestations.push(selfAttestation) From 2cfc55c0300aafa4c5478c68b4487d09139e8981 Mon Sep 17 00:00:00 2001 From: Domen Grabec Date: Thu, 27 Sep 2018 00:20:51 +0200 Subject: [PATCH 7/9] adding comments and abstracting constant --- src/resources/users.js | 4 ++++ src/services/contract-service.js | 7 ++++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/resources/users.js b/src/resources/users.js index af11f5bd..694e3fa8 100644 --- a/src/resources/users.js +++ b/src/resources/users.js @@ -5,6 +5,10 @@ class Users { this.resolver = new UsersResolver({ contractService, ipfsService }) } + /* possible options values: + * - confirmationCallback(confirmationCount, transactionReceipt) -> called repeatedly after a transaction is mined + * - transactionHashCallback(hash) -> called immediately when the transaction hash is received + */ async set({ profile, attestations = [], options = {}}) { return this.resolver.set({ profile, attestations, options }) } diff --git a/src/services/contract-service.js b/src/services/contract-service.js index ed1891e1..3d30daca 100644 --- a/src/services/contract-service.js +++ b/src/services/contract-service.js @@ -13,6 +13,7 @@ import bs58 from 'bs58' import Web3 from 'web3' const emptyAddress = '0x0000000000000000000000000000000000000000' +const NUMBER_CONFIRMATIONS_TO_REPORT = 20 const SUPPORTED_ERC20 = [ { symbol: 'OGN', decimals: 18, contractName: 'OriginToken' } ] @@ -220,7 +221,7 @@ class ContractService { } /* 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. + * so this function perpetually (until 20 confirmations) checks for presence of deployed contract. * * This could also be a problem in Ethereum node: https://github.com/ethereum/web3.js/issues/1255 */ @@ -238,8 +239,8 @@ class ContractService { confirmationCallback(confirmations, { transactionHash: transactionInfo.hash }) - // do checks untill 5 block confirmations - if (confirmations < 5) { + // do checks until NUMBER_CONFIRMATIONS_TO_REPORT block confirmations + if (confirmations < NUMBER_CONFIRMATIONS_TO_REPORT) { setTimeout(() => { this.checkForDeploymentCompletion(hash, confirmationCallback) }, 1500) From 697ac79d654bf8b21d24c9e77f79f6e07d5c2765 Mon Sep 17 00:00:00 2001 From: Domen Grabec Date: Thu, 27 Sep 2018 00:46:48 +0200 Subject: [PATCH 8/9] add test for transactionHash and onComplete callbacks --- test/resource_users.test.js | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/test/resource_users.test.js b/test/resource_users.test.js index 08210c9e..abd5c26e 100644 --- a/test/resource_users.test.js +++ b/test/resource_users.test.js @@ -1,4 +1,5 @@ import chai from 'chai' +import chaiString from 'chai-string' import chaiAsPromised from 'chai-as-promised' import Web3 from 'web3' @@ -9,6 +10,7 @@ import ContractService from '../src/services/contract-service' import IpfsService from '../src/services/ipfs-service' chai.use(chaiAsPromised) +chai.use(chaiString) const expect = chai.expect const issuerPrivatekey = @@ -220,6 +222,30 @@ describe('User Resource', function() { const badProfile = { profile: { bad: 'profile' } } return expect(users.set(badProfile)).to.eventually.be.rejectedWith(Error) }) + + it('should be able to receive transactionHash and onComplete callbacks', async () => { + let transactionHash, confCount, trReceipt + + await users.set({ + profile: { + firstName: 'Wonder', + lastName: 'Woman', + }, + options: { + transactionHashCallback: (hash) => { + transactionHash = hash + }, + confirmationCallback: (confirmationCount, transactionReceipt) => { + confCount = confirmationCount + trReceipt = transactionReceipt + } + } + }) + + expect(transactionHash).to.startsWith('0x') + expect(confCount).is.a('number') + expect(trReceipt).is.a('object') + }) }) describe('get', () => { From fa7b66f1fcec4f699b516542e8d59b1bf9229741 Mon Sep 17 00:00:00 2001 From: Domen Grabec Date: Thu, 27 Sep 2018 10:40:23 +0200 Subject: [PATCH 9/9] handle possible race condition in test --- test/resource_users.test.js | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/test/resource_users.test.js b/test/resource_users.test.js index abd5c26e..9de7cf80 100644 --- a/test/resource_users.test.js +++ b/test/resource_users.test.js @@ -223,10 +223,11 @@ describe('User Resource', function() { return expect(users.set(badProfile)).to.eventually.be.rejectedWith(Error) }) - it('should be able to receive transactionHash and onComplete callbacks', async () => { - let transactionHash, confCount, trReceipt + it('should be able to receive transactionHash and onComplete callbacks', (done) => { + let transactionHash + let doneCalled = false - await users.set({ + users.set({ profile: { firstName: 'Wonder', lastName: 'Woman', @@ -236,15 +237,19 @@ describe('User Resource', function() { transactionHash = hash }, confirmationCallback: (confirmationCount, transactionReceipt) => { - confCount = confirmationCount - trReceipt = transactionReceipt + expect(confirmationCount).is.a('number') + expect(transactionReceipt).is.a('object') + // transactionHashCallback should always execute before confirmationCallback + expect(transactionHash).to.startsWith('0x') + + // prevent done being called multiple times + if (!doneCalled){ + doneCalled = true + done() + } } } }) - - expect(transactionHash).to.startsWith('0x') - expect(confCount).is.a('number') - expect(trReceipt).is.a('object') }) })