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

Optional net_peerCount response format #377

Merged

Conversation

tgmichel
Copy link
Contributor

@tgmichel tgmichel commented May 6, 2021

There are inconsistencies on the response format expected when calling net_peerCount. Some tools/dapps expect decimal format, others hex.

This PR adds a bool peer_count_as_hex parameter to NetApi, so projects instantiating the handler at service level can decide which format to use. Defaulted to false in the template.

@tgmichel tgmichel requested a review from sorpaas as a code owner May 6, 2021 14:09
client/rpc/src/eth.rs Outdated Show resolved Hide resolved
Co-authored-by: Joshy Orndorff <JoshOrndorff@users.noreply.github.com>
@notlesh
Copy link
Contributor

notlesh commented May 6, 2021

@sorpaas Since we've gone back and forth on the return type for this, what do you think about this solution? We're hoping to put out a new moonbeam release soon and would love to be as close to master as possible.

@tgmichel tgmichel closed this May 6, 2021
@sorpaas
Copy link
Member

sorpaas commented May 6, 2021

The PR looks fine, but honestly I think if Ethereum dapps expect different types themselves, then that's not really much we can do. We just have to make sure by default we follow Geth's type definition.

@tgmichel
Copy link
Contributor Author

tgmichel commented May 6, 2021

The PR looks fine, but honestly I think if Ethereum dapps expect different types themselves, then that's not really much we can do. We just have to make sure by default we follow Geth's type definition.

Yeah, I will re-open tomorrow, we realized was missing untagged for serializing the enum correctly. I will also make sure that we default to Geth's format, thanks for pointing that out.

@tgmichel tgmichel reopened this May 7, 2021
@sorpaas sorpaas merged commit d1c7768 into polkadot-evm:master May 7, 2021
@tgmichel tgmichel deleted the tgm-peer-count-option branch April 1, 2022 09:54
abhijeetbhagat pushed a commit to web3labs/frontier that referenced this pull request Jan 11, 2023
* Optional `net_peerCount` response format

* Prettify

Co-authored-by: Joshy Orndorff <JoshOrndorff@users.noreply.github.com>

* More fixes

* Set hex format as default

* Serialize untagged

* Add test for NetApi

Co-authored-by: Joshy Orndorff <JoshOrndorff@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants