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

[DO-NOT-MERGE] Better loom compatibility with JSON RPC specification for Ethereum #242

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

eduardonunesp
Copy link
Contributor

@eduardonunesp eduardonunesp commented Mar 19, 2019

This PR targets to improve the LoomProvider compatibility with Web3 and Block Explorers that understanding Eth JSON RPC

  • Create interfaces based on Eth JSON RPC calls
  • Create a strict mode that removes any non-compatible JSON RPC calls
  • Testing with ReceiptsV2
  • Test blockNumber tx fix

* Better docs on loom provider responses
* Improved interfaces on loom provider
* Do not throw when cant find getCode
* Improving block 0 handling
* Added strict mode that remove non standard variables
@eduardonunesp eduardonunesp force-pushed the improving-loom-provider-compatibilities branch from ae913e4 to 99219e8 Compare March 21, 2019 20:10
@eduardonunesp eduardonunesp changed the title [WIP] Better docs on loom provider responses Better docs on loom provider responses Mar 21, 2019
@eduardonunesp eduardonunesp requested a review from enlight March 21, 2019 20:18
@eduardonunesp eduardonunesp changed the title Better docs on loom provider responses Better loom compatibility with JSON RPC specification for Ethereum Mar 21, 2019
@@ -832,17 +987,28 @@ export class LoomProvider {
}

private _createLogResult(log: EthFilterLog): IEthFilterLog {
const removed = log.getRemoved()
const blockTime = numberToHexLC(log.getBlockTime())
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like this was a decimal before, won't converting to hex break clients?

Copy link
Contributor Author

@eduardonunesp eduardonunesp Mar 26, 2019

Choose a reason for hiding this comment

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

Fixed, the potential break can exist only if the client marks as strict like loomProvider.strict = true

blockNumber: string
transactionHash: string
number: string | null
hash: string
Copy link
Contributor

Choose a reason for hiding this comment

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

Potentially breaking changes?

Copy link
Contributor Author

@eduardonunesp eduardonunesp Mar 26, 2019

Choose a reason for hiding this comment

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

Those are the values according to the specification, added back the non-strict values at the end of the interface

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed, the potential break can exist only if the client marks as strict like loomProvider.strict = true

e2e_support/loom.yaml Show resolved Hide resolved
@eduardonunesp eduardonunesp changed the title Better loom compatibility with JSON RPC specification for Ethereum [DO-NOT-MERGE] Better loom compatibility with JSON RPC specification for Ethereum Apr 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants