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.

Sign up for GitHub

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

Jump to bottom

Change v field in eth_getTransactionByHash and eth_getBlockByHash to be zeroHex instead of null #917

Merged
merged 2 commits into from
Feb 24, 2023
Merged

Change v field in eth_getTransactionByHash and eth_getBlockByHash to be zeroHex instead of null #917

merged 2 commits into from
Feb 24, 2023

Conversation

sergioaiobuilders
Copy link
Contributor

@sergioaiobuilders sergioaiobuilders commented Feb 17, 2023 •
edited
Loading

Description:

This PR changes the current eth.ts behaviour in getTransactionFromContractResult and getTransactionByHash to return 0x0 instead of null when v is null
Related issue(s):

Fixes #916

Notes for reviewer:

Checklist

  • Documented (Code comments, README, etc.)
  • Tested (unit, integration, etc.)

@sergioaiobuilders sergioaiobuilders mentioned this pull request Feb 17, 2023
Closed
@sergioaiobuilders sergioaiobuilders changed the title Change 'v' field to be zeroHex instead of null Change v field in eth_getTransactionByHash and eth_getBlockByHash to be zeroHex instead of null Feb 17, 2023
@Nana-EC Nana-EC added bug Something isn't working P2 labels Feb 17, 2023
@Nana-EC Nana-EC added this to the 0.19.0 milestone Feb 17, 2023
@AlfredoG87
Copy link
Collaborator

@sergioaiobuilders, Thank you for your contribution, by what I can see, it looks good.
Just one more request, could you also add a test that ensure v values are 0x0 when null?

sergioaiobuilders and others added 2 commits February 20, 2023 09:33
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@sergioaiobuilders
Copy link
Contributor Author

@AlfredoG87 I just added the test! Let me know if anything else is needed.

AlfredoG87
AlfredoG87 approved these changes Feb 20, 2023
View reviewed changes
Copy link
Collaborator

@AlfredoG87 AlfredoG87 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

georgi-l95
georgi-l95 approved these changes Feb 23, 2023
View reviewed changes
Copy link
Collaborator

@georgi-l95 georgi-l95 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG

@Nana-EC Nana-EC merged commit 68b23ac into hashgraph:main Feb 24, 2023
@Nana-EC
Copy link
Collaborator

Nana-EC commented Feb 24, 2023

Thanks for the contribution @sergioaiobuilders

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AlfredoG87 AlfredoG87 AlfredoG87 approved these changes

@Nana-EC Nana-EC Nana-EC approved these changes

@georgi-l95 georgi-l95 georgi-l95 approved these changes

@shemnon shemnon Awaiting requested review from shemnon

@david-bakin-sl david-bakin-sl Awaiting requested review from david-bakin-sl

@ebadiere ebadiere Awaiting requested review from ebadiere

@lukelee-sl lukelee-sl Awaiting requested review from lukelee-sl

Assignees

@sergioaiobuilders sergioaiobuilders

Labels
bug Something isn't working P2
Projects
Smart Contract Sprint Board
Archived in project
Milestone
0.19.0
Development

Successfully merging this pull request may close these issues.

Web3j 4.9.6 fails to deserialize 'v' field when calling eth_getBlockByNumber or eth_getTransactionByHash
4 participants
@sergioaiobuilders @AlfredoG87 @Nana-EC @georgi-l95