From d0448c2bbd90c6c103f07b201886670dc4675a43 Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Mon, 24 Sep 2018 15:02:06 +0100 Subject: Fix bug where if block wasn't found, getBlockAsync would throw. Now it returns `undefined` --- packages/web3-wrapper/src/web3_wrapper.ts | 26 ++++++++++++------- packages/web3-wrapper/test/web3_wrapper_test.ts | 34 +++++++++++++++++-------- 2 files changed, 40 insertions(+), 20 deletions(-) (limited to 'packages/web3-wrapper') diff --git a/packages/web3-wrapper/src/web3_wrapper.ts b/packages/web3-wrapper/src/web3_wrapper.ts index af0fe3a72..40e0a73f0 100644 --- a/packages/web3-wrapper/src/web3_wrapper.ts +++ b/packages/web3-wrapper/src/web3_wrapper.ts @@ -329,23 +329,28 @@ export class Web3Wrapper { /** * Fetch a specific Ethereum block without transaction data * @param blockParam The block you wish to fetch (blockHash, blockNumber or blockLiteral) - * @returns The requested block without transaction data + * @returns The requested block without transaction data, or undefined if block was not found */ - public async getBlockAsync(blockParam: string | BlockParam): Promise { + public async getBlockIfExistsAsync( + blockParam: string | BlockParam, + ): Promise { Web3Wrapper._assertBlockParamOrString(blockParam); const encodedBlockParam = marshaller.marshalBlockParam(blockParam); const method = utils.isHexStrict(blockParam) ? 'eth_getBlockByHash' : 'eth_getBlockByNumber'; const shouldIncludeTransactionData = false; - const blockWithoutTransactionDataWithHexValues = await this._sendRawPayloadAsync< + const blockWithoutTransactionDataWithHexValuesOrNull = await this._sendRawPayloadAsync< BlockWithoutTransactionDataRPC >({ method, params: [encodedBlockParam, shouldIncludeTransactionData], }); - const blockWithoutTransactionData = marshaller.unmarshalIntoBlockWithoutTransactionData( - blockWithoutTransactionDataWithHexValues, - ); - return blockWithoutTransactionData; + let blockWithoutTransactionDataIfExists; + if (!_.isNull(blockWithoutTransactionDataWithHexValuesOrNull)) { + blockWithoutTransactionDataIfExists = marshaller.unmarshalIntoBlockWithoutTransactionData( + blockWithoutTransactionDataWithHexValuesOrNull, + ); + } + return blockWithoutTransactionDataIfExists; } /** * Fetch a specific Ethereum block with transaction data @@ -376,8 +381,11 @@ export class Web3Wrapper { */ public async getBlockTimestampAsync(blockParam: string | BlockParam): Promise { Web3Wrapper._assertBlockParamOrString(blockParam); - const { timestamp } = await this.getBlockAsync(blockParam); - return timestamp; + const blockIfExists = await this.getBlockIfExistsAsync(blockParam); + if (_.isUndefined(blockIfExists)) { + throw new Error(`Failed to fetch block with blockParam: ${JSON.stringify(blockParam)}`); + } + return blockIfExists.timestamp; } /** * Retrieve the user addresses available through the backing provider diff --git a/packages/web3-wrapper/test/web3_wrapper_test.ts b/packages/web3-wrapper/test/web3_wrapper_test.ts index b4fd8bb44..385c469bf 100644 --- a/packages/web3-wrapper/test/web3_wrapper_test.ts +++ b/packages/web3-wrapper/test/web3_wrapper_test.ts @@ -85,28 +85,40 @@ describe('Web3Wrapper tests', () => { expect(typeof blockNumber).to.be.equal('number'); }); }); - describe('#getBlockAsync', () => { + describe('#getBlockIfExistsAsync', () => { it('gets block when supplied a valid BlockParamLiteral value', async () => { const blockParamLiteral = BlockParamLiteral.Earliest; - const block = await web3Wrapper.getBlockAsync(blockParamLiteral); - expect(block.number).to.be.equal(0); - expect(utils.isBigNumber(block.difficulty)).to.equal(true); - expect(_.isNumber(block.gasLimit)).to.equal(true); + const blockIfExists = await web3Wrapper.getBlockIfExistsAsync(blockParamLiteral); + if (_.isUndefined(blockIfExists)) { + throw new Error('Expected block to exist'); + } + expect(blockIfExists.number).to.be.equal(0); + expect(utils.isBigNumber(blockIfExists.difficulty)).to.equal(true); + expect(_.isNumber(blockIfExists.gasLimit)).to.equal(true); }); it('gets block when supplied a block number', async () => { const blockParamLiteral = 0; - const block = await web3Wrapper.getBlockAsync(blockParamLiteral); - expect(block.number).to.be.equal(0); + const blockIfExists = await web3Wrapper.getBlockIfExistsAsync(blockParamLiteral); + if (_.isUndefined(blockIfExists)) { + throw new Error('Expected block to exist'); + } + expect(blockIfExists.number).to.be.equal(0); }); it('gets block when supplied a block hash', async () => { const blockParamLiteral = 0; - const block = await web3Wrapper.getBlockAsync(blockParamLiteral); - const sameBlock = await web3Wrapper.getBlockAsync(block.hash as string); - expect(sameBlock.number).to.be.equal(0); + const blockIfExists = await web3Wrapper.getBlockIfExistsAsync(blockParamLiteral); + if (_.isUndefined(blockIfExists)) { + throw new Error('Expected block to exist'); + } + const sameBlockIfExists = await web3Wrapper.getBlockIfExistsAsync(blockIfExists.hash as string); + if (_.isUndefined(sameBlockIfExists)) { + throw new Error('Expected block to exist'); + } + expect(sameBlockIfExists.number).to.be.equal(0); }); it('should throw if supplied invalid blockParam value', async () => { const invalidBlockParam = 'deadbeef'; - expect(web3Wrapper.getBlockAsync(invalidBlockParam)).to.eventually.to.be.rejected(); + expect(web3Wrapper.getBlockIfExistsAsync(invalidBlockParam)).to.eventually.to.be.rejected(); }); }); describe('#getBlockWithTransactionDataAsync', () => { -- cgit v1.2.3 From ebddf82819a9fe9eb8550e939ea946248718beef Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Mon, 24 Sep 2018 15:02:47 +0100 Subject: Add CHANGELOG entry for change to getBlockAsync --- packages/web3-wrapper/CHANGELOG.json | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'packages/web3-wrapper') diff --git a/packages/web3-wrapper/CHANGELOG.json b/packages/web3-wrapper/CHANGELOG.json index 1653f388c..eb21dc3dc 100644 --- a/packages/web3-wrapper/CHANGELOG.json +++ b/packages/web3-wrapper/CHANGELOG.json @@ -1,4 +1,13 @@ [ + { + "version": "3.0.0", + "changes": [ + { + "note": + "Rename `getBlockAsync` to `getBlockIfExistsAsync` and rather then throw if the requested block wasn't found, return undefined." + } + ] + }, { "version": "2.0.3", "changes": [ -- cgit v1.2.3 From 5d88a56452d9f953e36a91991573ff0f3efa0da4 Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Mon, 24 Sep 2018 15:11:29 +0100 Subject: Add PR nr --- packages/web3-wrapper/CHANGELOG.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'packages/web3-wrapper') diff --git a/packages/web3-wrapper/CHANGELOG.json b/packages/web3-wrapper/CHANGELOG.json index eb21dc3dc..ce3f5e3be 100644 --- a/packages/web3-wrapper/CHANGELOG.json +++ b/packages/web3-wrapper/CHANGELOG.json @@ -4,7 +4,8 @@ "changes": [ { "note": - "Rename `getBlockAsync` to `getBlockIfExistsAsync` and rather then throw if the requested block wasn't found, return undefined." + "Rename `getBlockAsync` to `getBlockIfExistsAsync` and rather then throw if the requested block wasn't found, return undefined.", + "pr": 1082 } ] }, -- cgit v1.2.3 From 311b92591955e9b6eaaaffe6b92b9f33a05d38b4 Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Mon, 24 Sep 2018 15:14:07 +0100 Subject: Add to doc comment why a block might not be returned to the caller --- packages/web3-wrapper/src/web3_wrapper.ts | 1 + 1 file changed, 1 insertion(+) (limited to 'packages/web3-wrapper') diff --git a/packages/web3-wrapper/src/web3_wrapper.ts b/packages/web3-wrapper/src/web3_wrapper.ts index 40e0a73f0..dc634a57f 100644 --- a/packages/web3-wrapper/src/web3_wrapper.ts +++ b/packages/web3-wrapper/src/web3_wrapper.ts @@ -330,6 +330,7 @@ export class Web3Wrapper { * Fetch a specific Ethereum block without transaction data * @param blockParam The block you wish to fetch (blockHash, blockNumber or blockLiteral) * @returns The requested block without transaction data, or undefined if block was not found + * (e.g the node isn't fully synced, there was a block re-org and the requested block was uncles, etc...) */ public async getBlockIfExistsAsync( blockParam: string | BlockParam, -- cgit v1.2.3