Transactions pool endpoint #1165
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## development #1165 +/- ##
===============================================
+ Coverage 38.60% 38.80% +0.19%
===============================================
Files 9 9
Lines 316 317 +1
Branches 19 19
===============================================
+ Hits 122 123 +1
Misses 175 175
Partials 19 19
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
bogdan-rosianu
changed the title
There was a problem hiding this comment.
we also need here the possibility of filtering by type and also the /pool/count endpoint
There was a problem hiding this comment.
great idea. done
| @ApiQuery({ name: 'from', description: 'Number of items to skip for the result set', required: false }) | ||
| @ApiQuery({ name: 'size', description: 'Number of items to retrieve', required: false }) | ||
| async getTransactionPool( | ||
| @Query('from', new DefaultValuePipe(0), ParseIntPipe) from: number, |
There was a problem hiding this comment.
filter by sender / receiver should also be possible. same with the /count endpoint
| ): Promise<TransactionInPool> { | ||
| const transaction = await this.poolService.getTransactionFromPool(txHash); | ||
| if (transaction === undefined) { | ||
| throw new HttpException('Transaction not found', HttpStatus.NOT_FOUND); |
There was a problem hiding this comment.
you can throw here the dedicated NotFoundException
config/config.devnet.yaml
Outdated
| transactionPoolWarmer: | ||
| enabled: true | ||
| cronExpression: '*/5 * * * * *' | ||
| ttlInSeconds: 6 |
There was a problem hiding this comment.
default ttl should be maybe 60 seconds, to make sure the value doesn't expire if the gateway experiences a slowdown
There was a problem hiding this comment.
even if it's a bit verbose, it's a good best practice to have file names and class names 1:1, then it's easier to search for classes in vscode also by file name
There was a problem hiding this comment.
done. renamed to tx.pool.gateway.response.ts. I have multiple classes in that file, but the TxPoolGatewayResponse is the highest one
|
|
||
| @Injectable() | ||
| export class GatewayService { | ||
| private snapshotlessRequestsSet: Set<String> | undefined; |
There was a problem hiding this comment.
you can initialize this directly here. we can then make the field read-only and skip the undefined marker
src/endpoints/pool/pool.service.ts
Outdated
| private parseTransactions(rawPool: TxPoolGatewayResponse): TransactionInPool[] { | ||
| const transactionPool: TransactionInPool[] = []; | ||
| if (rawPool && rawPool.txPool) { | ||
| transactionPool.push(...this.processTransactionType(rawPool.txPool.regularTransactions || [], TransactionType.Transaction)); |
There was a problem hiding this comment.
instead of || [], please use the preferred ?? [] annotation which explicitly handles null or undefined
src/endpoints/pool/pool.service.ts
Outdated
|
|
||
| private parseTransaction(tx: TxInPoolFields, type: TransactionType): TransactionInPool { | ||
| return new TransactionInPool({ | ||
| txHash: tx.hash || '', |
There was a problem hiding this comment.
similar to above, use the preferred ?? operator
src/endpoints/pool/pool.service.ts
Outdated
|
|
||
| const results: TransactionInPool[] = []; | ||
|
|
||
| for (const transaction of pool) { |
There was a problem hiding this comment.
instead of using the for and continue strategy, we can use the .filter function for every filter parameter. in this way, we can skip the initial check which might cause bugs when adding a new filter
There was a problem hiding this comment.
refactored by using the filter function
bogdan-rosianu
dismissed stale reviews from ramonalobont and cfaur09
via
5e1f8d9
There was a problem hiding this comment.
can you please split all classes here in multiple files? easier to search and match
… SERVICES-2012-txs-pool-api
Reasoning
Proposed Changes
/poolendpoint that can return the transactions pool by also using a cache warmer (configurable via flag) to continuously fetch the new transactions/pool/:txHashendpoint that can return transaction pool details for a given txHash/pool/countendpoint that can return total transactions poolsender, receiver, type)How to test
<api>:3001/pool-> should return all transactions from pool<api>:3001/pool?fields=receiver,value-> should return all transactions from pool withreceiver and valueproperties<api>:3001/pool?from=0&size=15-> should return 15 transactions from pool<api>:3001/pool?fields=nonce&from=0&size=20-> should return 20 transactions from pool withnonceproperty only<api>:3001/pool/ac9eab8e2e6bc18fb241fce28d6ef949a8422668c9b0b8a47ae65ca80bc6bd40-> should return transaction pool details for the given txHash<api>:3001/pool/count-> should return total transactions pool<api>:3001/pool?type=Reward-> should return all Reward transactions from pool