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

Txpool module json responses #771

Merged
merged 2 commits into from Feb 28, 2019

Conversation

Projects
None yet
5 participants
@M-Picco
Copy link
Contributor

commented Feb 27, 2019

Addresses #689.

  • Fix escaped-string response by returning a Jackson JsonNode object instead of a String
  • Fix serialization issues through the use of the TypeConverter class
@tinchou
Copy link
Contributor

left a comment

Have you checked that TypeConverter.toJsonHex for every value is the right encoding? Other than that, looks great.

M-Picco added some commits Feb 26, 2019

Return JsonNode from TxPoolModule methods instead of a String in orde…
…r to return an actual JSON object instead of a escaped string
Fix serialization issues (non-camelcase names, non-hexstring values) …
…in TxPoolModule::serializeTransactions so that it complies with the JSON RPC standard

@M-Picco M-Picco force-pushed the txpool_module_json_responses branch from 4e92275 to 998f70d Feb 28, 2019

@rskops

This comment has been minimized.

Copy link

commented Feb 28, 2019

SonarQube analysis reported 9 issues

  1. MINOR Web3TxPoolModule.java#L26: Rename this method name to match the regular expression '^[a-z][a-zA-Z0-9]*$'. rule
  2. MINOR Web3TxPoolModule.java#L30: Rename this method name to match the regular expression '^[a-z][a-zA-Z0-9]*$'. rule
  3. MINOR Web3TxPoolModule.java#L34: Rename this method name to match the regular expression '^[a-z][a-zA-Z0-9]*$'. rule
  4. MINOR TxPoolModuleImpl.java#L67: Immediately return this expression instead of assigning it to the temporary variable "node". rule
  5. MINOR TxPoolModuleImpl.java#L152: Immediately return this expression instead of assigning it to the temporary variable "node". rule
  6. MINOR TxPoolModuleImpl.java#L168: Immediately return this expression instead of assigning it to the temporary variable "node". rule
  7. INFO TxPoolModuleImpl.java#L68: Method co.rsk.rpc.modules.txpool.TxPoolModuleImpl.content() stores return result in local before immediately returning it rule
  8. INFO TxPoolModuleImpl.java#L153: Method co.rsk.rpc.modules.txpool.TxPoolModuleImpl.inspect() stores return result in local before immediately returning it rule
  9. INFO TxPoolModuleImpl.java#L169: Method co.rsk.rpc.modules.txpool.TxPoolModuleImpl.status() stores return result in local before immediately returning it rule
@M-Picco

This comment has been minimized.

Copy link
Contributor Author

commented Feb 28, 2019

I'm pretty sure most values should be hex encoded and prefixed with '0x'.

I tried to follow the same guidelines as in ethereum

@tinchou
Copy link
Contributor

left a comment

OK sounds good!

@lsebrie lsebrie merged commit 422b6b9 into master Feb 28, 2019

3 checks passed

ci/circleci Your tests passed on CircleCI!
Details
default Build finished.
Details
sonarqube SonarQube reported 9 issues, no criticals or blockers

@lsebrie lsebrie deleted the txpool_module_json_responses branch Feb 28, 2019

diega added a commit that referenced this pull request Mar 1, 2019

@aeidelman aeidelman added this to the Orchid v0.6.2 milestone Apr 19, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.