Skip to content

Commit 6c9c59b

Browse files
authored
fix: Error block XXX not found (#306)
* Remove unused console log * More accurate message for block not found * Add verification for existence of the last block timestamp * Reset timeout sync to 10000ms * Remove accept json * Set confirmation number to 2 * Increase timeout for tests concerned by ethereal-storage
1 parent 70e5e30 commit 6c9c59b

File tree

10 files changed

+45
-46
lines changed

10 files changed

+45
-46
lines changed

packages/data-access/src/timestamp-by-location.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,9 @@ export default class LocationTimestamp {
2222
* @param timestamp timestamp of the block
2323
*/
2424
public pushTimestampByLocation(dataId: string, timestamp: number): void {
25-
if (this.timestampLocations[dataId]) {
26-
throw Error(`Timestamp already know for the dataId ${dataId}`);
25+
if (!this.timestampLocations[dataId]) {
26+
this.timestampLocations[dataId] = timestamp;
2727
}
28-
this.timestampLocations[dataId] = timestamp;
2928
}
3029

3130
/**

packages/data-access/test/timestamp-by-location.test.ts

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,6 @@ describe('LocationTimestamp', () => {
1616
const result = timestampLocation.getTimestampFromLocation(arbitraryDataId1);
1717
expect(result, 'timestampLocation is wrong').to.equal(arbitraryTimestamp);
1818
});
19-
it('cannot pushTimestampByLocation() a dataId already pushed', () => {
20-
const timestampLocation = new TimestampLocation();
21-
timestampLocation.pushTimestampByLocation(arbitraryDataId1, arbitraryTimestamp);
22-
23-
expect(() => {
24-
timestampLocation.pushTimestampByLocation(arbitraryDataId1, arbitraryTimestamp);
25-
}, 'must throw').to.throw(`Timestamp already know for the dataId ${arbitraryDataId1}`);
26-
});
2719

2820
describe('isDataInBoundaries', () => {
2921
it('can isDataInBoundaries()', () => {

packages/ethereum-storage/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@
4444
"lint-staged": "lint-staged",
4545
"ganache": "ganache-cli -l 90000000 -p 8545 -m \"candy maple cake sugar pudding cream honey rich smooth crumble sweet treat\"",
4646
"deploy": "truffle --contracts_directory=./src deploy",
47-
"test:lib": "nyc mocha --require ts-node/register --require source-map-support/register \"test/lib/**/*.ts\"",
47+
"test:lib": "nyc mocha --timeout=5000 --require ts-node/register --require source-map-support/register \"test/lib/**/*.ts\"",
4848
"test:sol": "truffle test --contracts_directory=./src test/contracts/requestHashStorage.js",
4949
"test": "yarn run test:lib && yarn run test:sol",
5050
"coverage": "nyc report --reporter=text-lcov > coverage.lcov"

packages/ethereum-storage/src/lib/ethereum-blocks.ts

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -61,14 +61,22 @@ export default class EthereumBlocks {
6161
public async getBlockNumbersFromTimestamp(
6262
timestamp: number,
6363
): Promise<Types.IBlockNumbersInterval> {
64+
6465
// check if we have the blockTimestamp of the first significant block number
6566
if (!this.blockTimestamp[this.firstSignificantBlockNumber]) {
6667
// update the blockTimestamp cache with the first significant block
6768
await this.getBlockTimestamp(this.firstSignificantBlockNumber);
6869
}
70+
6971
// update the last block number in memory
7072
const lastBlockNumber: number = await this.getLastBlockNumber();
7173

74+
// check if we have the blockTimestamp of the last number
75+
if (!this.blockTimestamp[lastBlockNumber]) {
76+
// update the blockTimestamp cache with the last block
77+
await this.getBlockTimestamp(lastBlockNumber);
78+
}
79+
7280
// if timestamp before first significant block, return the significant block
7381
if (timestamp <= this.blockTimestamp[this.firstSignificantBlockNumber]) {
7482
return {
@@ -77,9 +85,12 @@ export default class EthereumBlocks {
7785
};
7886
}
7987

80-
// if timestamp after last block, return null
88+
// if timestamp after last block, return lastBlockNumber
8189
if (timestamp > this.blockTimestamp[lastBlockNumber]) {
82-
throw Error('Timestamp is bigger than the last block of ethereum');
90+
return {
91+
blockAfter: lastBlockNumber,
92+
blockBefore: lastBlockNumber,
93+
};
8394
}
8495

8596
// Before doing the dichotomic search, we restrict the search to the two closest block we already know
@@ -101,12 +112,7 @@ export default class EthereumBlocks {
101112
* @return blockNumber of the last block
102113
*/
103114
public async getLastBlockNumber(): Promise<number> {
104-
const lastBlockNumber = await this.eth.getBlockNumber();
105-
106-
// Store the timestamp of the block
107-
await this.getBlockTimestamp(lastBlockNumber);
108-
109-
return lastBlockNumber;
115+
return this.eth.getBlockNumber();
110116
}
111117

112118
/**

packages/ethereum-storage/src/lib/smart-contract-manager.ts

Lines changed: 13 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ const web3Eth = require('web3-eth');
77

88
const bigNumber: any = require('bn.js');
99

10+
// Confirmation to wait in order to create metadata of the new added hash
11+
// TODO(PROT-252): Find the optimal value or fix to use 1
12+
const CONFIRMATION_TO_WAIT = 2;
13+
1014
/**
1115
* Manages the smart contract used by the storage layer
1216
* to store the hashes of the data on Ethereum
@@ -82,7 +86,7 @@ export default class SmartContractManager {
8286
// Get the accounts on the provider
8387
// Throws an error if timeout is reached
8488
const accounts = await Promise.race([
85-
this.timeoutPromise(this.timeout, 'Web3 provider connection timeout'),
89+
this.timeoutPromise(this.timeout, 'Web3 getAccounts connection timeout'),
8690
this.eth.getAccounts(),
8791
]);
8892

@@ -110,13 +114,15 @@ export default class SmartContractManager {
110114
// Get the fee from the size of the content
111115
// Throws an error if timeout is reached
112116
const fee = await Promise.race([
113-
this.timeoutPromise(this.timeout, 'Web3 provider connection timeout'),
117+
this.timeoutPromise(this.timeout, 'Web3 getFeesAmount connection timeout'),
114118
this.requestHashStorage.methods.getFeesAmount(contentSize).call(),
115119
]);
116120

117121
const gasPriceToUse = gasPrice || config.getDefaultEthereumGasPrice();
118122

119123
// Send transaction to contract
124+
// TODO(PROT-181): Implement a log manager for the library
125+
// use it for the different events (error, transactionHash, receipt and confirmation)
120126
return new Promise(
121127
(resolve, reject): any => {
122128
this.requestHashStorage.methods
@@ -130,22 +136,9 @@ export default class SmartContractManager {
130136
.on('error', (transactionError: string) => {
131137
reject(Error(`Ethereum transaction error: ${transactionError}`));
132138
})
133-
.on('transactionHash', (transactionHash: string) => {
134-
// TODO(PROT-181): Implement a log manager for the library
135-
/* tslint:disable:no-console */
136-
console.log(`transactionHash : ${transactionHash}`);
137-
})
138-
.on('receipt', (receiptInCallback: string) => {
139-
// TODO(PROT-181): Implement a log manager for the library
140-
/* tslint:disable:no-console */
141-
console.log(`receipt : ${JSON.stringify(receiptInCallback)}`);
142-
})
143139
.on('confirmation', (confirmationNumber: number, receiptAfterConfirmation: any) => {
144-
// TODO(PROT-181): Implement a log manager for the library
145-
// TODO(PROT-252): return after X confirmation instead of 0
146-
147-
// We have to wait at least one confirmation to get Ethereum metadata
148-
if (confirmationNumber > 0) {
140+
// TODO(PROT-252): search for the best number of confirmation to wait for
141+
if (confirmationNumber >= CONFIRMATION_TO_WAIT) {
149142
const gasFee = new bigNumber(receiptAfterConfirmation.gasUsed).mul(
150143
new bigNumber(gasPriceToUse),
151144
);
@@ -234,7 +227,7 @@ export default class SmartContractManager {
234227

235228
// Reading all event logs
236229
let events = await Promise.race([
237-
this.timeoutPromise(this.timeout, 'Web3 provider connection timeout'),
230+
this.timeoutPromise(this.timeout, 'Web3 getPastEvents connection timeout'),
238231
this.requestHashStorage.getPastEvents({
239232
event: 'NewHash',
240233
fromBlock,
@@ -338,7 +331,8 @@ export default class SmartContractManager {
338331
try {
339332
blockTimestamp = await this.ethereumBlocks.getBlockTimestamp(blockNumber);
340333
} catch (error) {
341-
throw Error(`Error getting block timestamp: ${error}`);
334+
335+
throw Error(`Error getting block ${blockNumber} timestamp: ${error}`);
342336
}
343337

344338
return {

packages/ethereum-storage/test/lib/ethereum-blocks.test.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -245,10 +245,11 @@ describe('EthereumBlocks', () => {
245245
blockBefore: 99,
246246
});
247247

248-
// after last block
249-
await expect(ethereumBlocks.getBlockNumbersFromTimestamp(99999)).to.eventually.rejectedWith(
250-
'Timestamp is bigger than the last block of ethereum',
251-
);
248+
// with timestamp over last block
249+
expect(await ethereumBlocks.getBlockNumbersFromTimestamp(99999)).to.be.deep.equal({
250+
blockAfter: 99,
251+
blockBefore: 99,
252+
});
252253
});
253254
});
254255
});

packages/integration-test/package.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,8 @@
3131
"lint": "tslint --project . && eslint \"test/**/*.ts\"",
3232
"lint-staged": "lint-staged",
3333
"test": "run-s test:node test:layers",
34-
"test:layers": "mocha --timeout=5000 --require ts-node/register \"test/layers.test.ts\"",
35-
"test:node": "mocha --timeout=5000 --require ts-node/register \"test/node-client.test.ts\""
34+
"test:layers": "mocha --timeout=10000 --require ts-node/register \"test/layers.test.ts\"",
35+
"test:node": "mocha --timeout=10000 --require ts-node/register \"test/node-client.test.ts\""
3636
},
3737
"devDependencies": {
3838
"@requestnetwork/advanced-logic": "0.1.1-alpha.7",

packages/request-node/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
],
3131
"scripts": {
3232
"build": "tsc -b",
33-
"test": "nyc mocha --timeout=4000 --exit --require ts-node/register --require source-map-support/register test/**/*.ts",
33+
"test": "nyc mocha --timeout=10000 --exit --require ts-node/register --require source-map-support/register test/**/*.ts",
3434
"start": "ts-node src/server.ts",
3535
"clean": "shx rm -rf dist",
3636
"lint-staged": "lint-staged",

packages/request-node/src/request/getTransactionsByTopic.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,9 @@ export default async function getTransactionsByTopic(
4242

4343
serverResponse.status(httpStatus.OK).send(transactions);
4444
} catch (e) {
45+
// tslint:disable-next-line:no-console
46+
console.error(`getTransactionsByTopic error: ${e}`);
47+
4548
serverResponse.status(httpStatus.INTERNAL_SERVER_ERROR).send(e);
4649
}
4750
}

packages/request-node/src/request/persistTransaction.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,11 @@ export default async function persistTransaction(
3131
);
3232

3333
serverResponse.status(httpStatus.OK).send(dataAccessResponse);
34+
3435
} catch (e) {
36+
// tslint:disable-next-line:no-console
37+
console.error(`persistTransaction error: ${e}`);
38+
3539
serverResponse.status(httpStatus.INTERNAL_SERVER_ERROR).send(e);
3640
}
3741
}

0 commit comments

Comments
 (0)