Skip to content

Conversation

digitaldonkey
Copy link

I found it very confusing that the boolean "default block parameter required" setting is actually implemented as a constant "2".

https://github.com/ethereum/wiki/wiki/JSON-RPC#the-default-block-parameter

@SilentCicero
Copy link
Member

ah, okay let me review this further. Sounds reasonable thanks @digitaldonkey sorry, been traveling, haven't had time to review.

@digitaldonkey
Copy link
Author

Would be great if we get in sync again. I based a PHP Library on the schema.

@SilentCicero
Copy link
Member

@digitaldonkey that's great, glad to see it being used. I'll look into switching to your more clear Bool. Sorry just traveling right now.

```
methods: {
<method name : [ input(s), output(s), minimum required outputs, 'latest' tag default position (if any) ] >,
<method name : [ Array Input(s), Array Output(s), String Output type, Integer Minimum required parameters, Boolean Requires default block parameter] >,
Copy link

Choose a reason for hiding this comment

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

I believe the Array Output(s) is unnecessary, no? The second element is the String Output Type?

Copy link
Member

Choose a reason for hiding this comment

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

I might want to look into that. Yes, you may be right.

Copy link

@sunny-g sunny-g Jun 7, 2017

Choose a reason for hiding this comment

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

I misspoke - the second element should be Array Output(s), and so I the String Output Type is extraneous (e.g. eth_getFilterLogs returns ["FilterChange"].

@sunny-g
Copy link

sunny-g commented Jun 4, 2017

Can this be merged? I'm building a library in Elixir built off of this schema as well 😄

@SilentCicero
Copy link
Member

@sunny-g it would mean I need to re-work another library as well (i.e. ethjs-query). If a PR can be made to change that also, then yes. I wont be able to get to this till Wednesday at the earliest.

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.

3 participants