Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Format API results inside _wrap* methods #3818

Closed
wants to merge 1 commit into from
Closed

Format API results inside _wrap* methods #3818

wants to merge 1 commit into from

Conversation

ly0va
Copy link

@ly0va ly0va commented Feb 22, 2023

There are 4 wrapping methods in AbstractProvider:

  • _wrapBlock
  • _wrapLog
  • _wrapTransactionReceipt
  • _wrapTransactionResponse

For some reason, formatTransactionResponse was not being called in _wrapTransactionResponse, although the analogous functions were being called in other wrapping methods.

Also, for some reason, formatting functions were being called on the arguments of wrapping methods before being passed in, even though they are called inside as well.

This PR fixes the inconsistencies.

The argument for formatting functions to be called inside wrapping methods is that if we want to override the formatters in the derived Provider class (say, if a network has extra fields in Block or TransactionReceipt), it is much easier to do so by overriding wrapping methods than by overriding every method that calls wrapping methods.

@ricmoo ricmoo added on-deck This Enhancement or Bug is currently being worked on. v6 Issues regarding v6 labels Feb 27, 2023
@ricmoo
Copy link
Member

ricmoo commented Mar 20, 2023

This has been merged in v6.2.0.

Thanks! :)

@ricmoo ricmoo closed this Mar 20, 2023
@ricmoo ricmoo added fixed/complete This Bug is fixed or Enhancement is complete and published. and removed on-deck This Enhancement or Bug is currently being worked on. labels Mar 20, 2023
Woodpile37 pushed a commit to Woodpile37/ethers.js that referenced this pull request Jan 14, 2024
Woodpile37 pushed a commit to Woodpile37/ethers.js that referenced this pull request Jan 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixed/complete This Bug is fixed or Enhancement is complete and published. v6 Issues regarding v6
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants