From 940d84a7ee258bd1d8301c323573101f0800570d Mon Sep 17 00:00:00 2001 From: Jasmine/kimjimin Date: Fri, 14 Jan 2022 11:04:21 +0900 Subject: [PATCH 1/4] Copy the parameter to avoid race condition --- .../src/subscription.js | 6 +- test/subscription.js | 73 +++++++++++++++---- 2 files changed, 64 insertions(+), 15 deletions(-) diff --git a/packages/caver-core-subscriptions/src/subscription.js b/packages/caver-core-subscriptions/src/subscription.js index e1a14da6..ac2e1e2b 100644 --- a/packages/caver-core-subscriptions/src/subscription.js +++ b/packages/caver-core-subscriptions/src/subscription.js @@ -235,10 +235,14 @@ Subscription.prototype.subscribe = function() { isFinite(payload.params[1].fromBlock) ) { // send the subscription request + + // copy the params to avoid race-condition with deletion below this block + const blockParams = Object.assign({}, payload.params[1]); + this.options.requestManager.send( { method: 'klay_getLogs', - params: [payload.params[1]], + params: [blockParams], }, function(err, logs) { if (!err) { diff --git a/test/subscription.js b/test/subscription.js index 9be820e5..05928aa7 100644 --- a/test/subscription.js +++ b/test/subscription.js @@ -26,6 +26,8 @@ const caver = new Caver(websocketURL) let senderPrvKey let senderAddress let receiver +let contractAddress +let contractAbi // If you are using websocket provider, subscribe the 'newBlockHeaders' event through the subscriptions object after sending the transaction. // When receiving the 'newBlockHeaders' event, it queries the transaction receipt. @@ -42,19 +44,38 @@ let receiver // -> [request] klay_getTransactionReceipt // -> [response] receipt -before(() => { - senderPrvKey = - process.env.privateKey && String(process.env.privateKey).indexOf('0x') === -1 - ? `0x${process.env.privateKey}` - : process.env.privateKey +async function prepareContractTesting() { + const kip7 = await caver.kct.kip7.deploy({ + name: 'Jamie', + symbol: 'JME', + decimals: 18, + initialSupply: '1000000000000' + }, senderAddress) + contractAddress = kip7.options.address + contractAbi = kip7.options.jsonInterface - senderAddress = caver.klay.accounts.wallet.add(senderPrvKey).address + return kip7.transfer(receiver.address, 1000, { from: senderAddress }) +} - receiver = caver.klay.accounts.wallet.add(caver.klay.accounts.create()) -}) +describe('subscription should work well with websocket connection', () => { + before(function(done) { + this.timeout(200000) + + senderPrvKey = + process.env.privateKey && String(process.env.privateKey).indexOf('0x') === -1 + ? `0x${process.env.privateKey}` + : process.env.privateKey + + senderAddress = caver.klay.accounts.wallet.add(senderPrvKey).address + caver.wallet.add(caver.wallet.keyring.createFromPrivateKey(senderPrvKey)) + + receiver = caver.klay.accounts.wallet.add(caver.klay.accounts.create()) + caver.wallet.add(caver.wallet.keyring.createFromPrivateKey(receiver.privateKey)) -describe('get transaction', () => { - it('CAVERJS-UNIT-ETC-094: getTransaction should return information of transaction.', async () => { + prepareContractTesting().then(() => done()) + }) + + it('CAVERJS-UNIT-ETC-094: sendTransaction should return a transaction receipt.', async () => { const txObj = { from: senderAddress, to: receiver.address, @@ -83,17 +104,41 @@ describe('get transaction', () => { expect(receipt.typeInt).not.to.undefined expect(receipt.value).not.to.undefined }).timeout(10000) -}) -describe('Subscribe test throught contract event', () => { it('CAVERJS-UNIT-ETC-262: should emit subscription id when subscription is created', done => { - caver.wallet.add(caver.wallet.keyring.createFromPrivateKey(senderPrvKey)).address caver.kct.kip17.deploy({ name: 'Jasmine', symbol: 'JAS' }, senderAddress).then(deployed => { deployed.events.MinterAdded({}).on('connected', subscriptionId => { expect(subscriptionId).not.to.be.undefined - caver.currentProvider.connection.close() done() }) }) }).timeout(30000) + + // Regression test for a race-condition where a fresh caver instance + // subscribing to past events would have its call parameters deleted while it + // made initial Websocket handshake and return an incorrect response. + it('CAVERJS-UNIT-ETC-: should immediately listen for events in the past', async () => { + const freshCaver = new Caver(websocketURL) + const contract = freshCaver.contract.create(contractAbi, contractAddress) + + let counter = 0; + const latestBlock = await caver.rpc.klay.getBlockNumber() + caver.currentProvider.connection.close() + + await new Promise(async resolve => { + contract.events.allEvents({ + fromBlock: 0 + }) + .on('data', function(event) { + counter++; + expect(event.blockNumber < latestBlock).to.be.true + + if (counter === 2){ + this.removeAllListeners() + freshCaver.currentProvider.connection.close() + resolve() + } + }) + }) + }).timeout(30000) }) From f8355d6503b1a8b136f75bf22c91a156ea26afcf Mon Sep 17 00:00:00 2001 From: Jasmine/kimjimin Date: Fri, 14 Jan 2022 11:04:29 +0900 Subject: [PATCH 2/4] Updated versio to v1.6.7 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index e175a559..7054c7b9 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "caver-js", - "version": "1.6.6", + "version": "1.6.7", "description": "caver-js is a JavaScript API library that allows developers to interact with a Klaytn node", "main": "index.js", "types": "types/index.d.ts", From 9b2d882fb5588532283da0f8e29591d34133cc92 Mon Sep 17 00:00:00 2001 From: Jasmine/kimjimin Date: Fri, 14 Jan 2022 11:07:26 +0900 Subject: [PATCH 3/4] Added TC numbering --- test/subscription.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/subscription.js b/test/subscription.js index 05928aa7..289567c8 100644 --- a/test/subscription.js +++ b/test/subscription.js @@ -117,7 +117,7 @@ describe('subscription should work well with websocket connection', () => { // Regression test for a race-condition where a fresh caver instance // subscribing to past events would have its call parameters deleted while it // made initial Websocket handshake and return an incorrect response. - it('CAVERJS-UNIT-ETC-: should immediately listen for events in the past', async () => { + it('CAVERJS-UNIT-ETC-398: should immediately listen for events in the past', async () => { const freshCaver = new Caver(websocketURL) const contract = freshCaver.contract.create(contractAbi, contractAddress) From 73baebc7500a65717e4c8ef5471d3d4502c4df1e Mon Sep 17 00:00:00 2001 From: Jasmine/kimjimin Date: Fri, 14 Jan 2022 11:12:09 +0900 Subject: [PATCH 4/4] Fixed the lint errors --- .../src/subscription.js | 2 +- test/subscription.js | 46 ++++++++++--------- 2 files changed, 26 insertions(+), 22 deletions(-) diff --git a/packages/caver-core-subscriptions/src/subscription.js b/packages/caver-core-subscriptions/src/subscription.js index ac2e1e2b..557ea1c4 100644 --- a/packages/caver-core-subscriptions/src/subscription.js +++ b/packages/caver-core-subscriptions/src/subscription.js @@ -237,7 +237,7 @@ Subscription.prototype.subscribe = function() { // send the subscription request // copy the params to avoid race-condition with deletion below this block - const blockParams = Object.assign({}, payload.params[1]); + const blockParams = { ...payload.params[1] } this.options.requestManager.send( { diff --git a/test/subscription.js b/test/subscription.js index 289567c8..12308eb5 100644 --- a/test/subscription.js +++ b/test/subscription.js @@ -45,12 +45,15 @@ let contractAbi // -> [response] receipt async function prepareContractTesting() { - const kip7 = await caver.kct.kip7.deploy({ - name: 'Jamie', - symbol: 'JME', - decimals: 18, - initialSupply: '1000000000000' - }, senderAddress) + const kip7 = await caver.kct.kip7.deploy( + { + name: 'Jamie', + symbol: 'JME', + decimals: 18, + initialSupply: '1000000000000', + }, + senderAddress + ) contractAddress = kip7.options.address contractAbi = kip7.options.jsonInterface @@ -121,24 +124,25 @@ describe('subscription should work well with websocket connection', () => { const freshCaver = new Caver(websocketURL) const contract = freshCaver.contract.create(contractAbi, contractAddress) - let counter = 0; + let counter = 0 const latestBlock = await caver.rpc.klay.getBlockNumber() caver.currentProvider.connection.close() - await new Promise(async resolve => { - contract.events.allEvents({ - fromBlock: 0 - }) - .on('data', function(event) { - counter++; - expect(event.blockNumber < latestBlock).to.be.true - - if (counter === 2){ - this.removeAllListeners() - freshCaver.currentProvider.connection.close() - resolve() - } - }) + await new Promise(resolve => { + contract.events + .allEvents({ + fromBlock: 0, + }) + .on('data', function(event) { + counter++ + expect(event.blockNumber < latestBlock).to.be.true + + if (counter === 2) { + this.removeAllListeners() + freshCaver.currentProvider.connection.close() + resolve() + } + }) }) }).timeout(30000) })