Fix getTransactions web3.js method #26099
Conversation
kunaljaydesai
changed the title
| @@ -3566,7 +3566,16 @@ export class Connection { | |||
| if ('error' in res) { | |||
| throw new Error('failed to get transactions: ' + res.error.message); | |||
| } | |||
| return res.result; | |||
There was a problem hiding this comment.
Wow. Somehow TypeScript, despite the fact that it knows the type of res.result, typed the outer resasany`.
I wonder if this is because they have the same name (the variable name is shadowed).
There was a problem hiding this comment.
Found out the root cause of this. #26102
| return { | ||
| ...result, | ||
| transaction: { | ||
| ...result.transaction, | ||
| message: new Message(result.transaction.message), | ||
| }, | ||
| }; |
There was a problem hiding this comment.
Slight preference for pulling this out into a method (getTransactionResponseFromGetTransactionsResult()so thatgetTransactionandgetTransactions` can share it, but no big deal – what you've done here is more readable.
web3.js/test/connection.test.ts
Outdated
| @@ -2197,6 +2197,138 @@ describe('Connection', function () { | |||
| expect(nullResponse).to.be.null; | |||
| }); | |||
|
|
|||
| it('get transactions', async () => { | |||
steveluscher
force-pushed
the
kunal/fix-batch-transactions
branch
from
64d5ae5
to
5848e7b
Compare
mergify
bot
dismissed
steveluscher’s stale review
Pull request has been modified.
Codecov Report
@@ Coverage Diff @@
## master #26099 +/- ##
===========================================
- Coverage 82.1% 75.7% -6.4%
===========================================
Files 628 40 -588
Lines 171471 2348 -169123
Branches 0 339 +339
===========================================
- Hits 140878 1779 -139099
+ Misses 30593 450 -30143
- Partials 0 119 +119 |
Problem
getTransactionsparses the transaction response differently fromgetTransaction, but they return the same type. Specifically,transaction.messageis not of the typeMessage.Summary of Changes
Pulls out the correct
messagetype for each transaction ingetTransactionscallcc @joncinque would you be able to review this for me? thanks!