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

Invalid contract output parsing #443

Closed
karalabe opened this issue Apr 28, 2016 · 4 comments
Closed

Invalid contract output parsing #443

karalabe opened this issue Apr 28, 2016 · 4 comments
Labels
Bug Addressing a bug

Comments

@karalabe
Copy link
Contributor

I have the following output ABI definition for a contract call:

[{"name":"major","type":"uint32"},{"name":"minor","type":"uint32"},{"name":"patch","type":"uint32"},{"name":"commit","type":"bytes20"},{"name":"pass","type":"address[]"},{"name":"fail","type":"address[]"}]

Calling the specific method returns the following EVM output:

000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000400000000000000000000000000000000000000000000000000000000000000000000000000000000000000c000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000000001000000000000000000000000945e4c7991a3d210c5efc1a2cb8feeabad7cb9ca0000000000000000000000000000000000000000000000000000000000000000

Web3 (at least the one included in Geth) parses this as:

[1, 2, 4, "0x0000000000000000000000000000000000000004", ["0x0000000000000000000000000000000000000002"], []]

Whereas the Go ABI parser parses it (corrently) as:

{"Major": 1, "Minor": 2, "Patch": 4, "Commit": "0x0000000000000000000000000000000000000004", "Pass": ["0x945e4c7991a3d210c5efc1a2cb8feeabad7cb9ca"], "Fail": []}

@karalabe
Copy link
Contributor Author

Btw, you can also test this yourself on the testnet with this abi against the contract deployed at 0x48a117313039b73ab02fb9f73e04a66defe123ec, by calling the proposedVersion() method.

@frozeman
Copy link
Contributor

We currently not returning the named return values, but as an array. Though the address doesn't seem to be parsed correctly.

This is something we can look into once we start refactoring the decoding/encoding part

@frozeman frozeman added this to the 1.0 milestone Apr 28, 2016
@axic
Copy link
Contributor

axic commented Apr 29, 2016

Not meaning to hijack this, but I was wondering if ethereumjs-abi (which is a from scratch implementation based on the wiki) parses it properly (= the way Go does), and it does:

[ <BN: 1>,
  <BN: 2>,
  <BN: 4>,
  <Buffer 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 04>,
  [ <BN: 945e4c7991a3d210c5efc1a2cb8feeabad7cb9ca> ],
  [] ]

So it must be a bug in web3.js

@frozeman
Copy link
Contributor

Fixed in web3.js 1.0:

Result {
  '0': '1',
  '1': '2',
  '2': '4',
  '3': '0x0000000000000000000000000000000000000004',
  '4': [ '0x945E4C7991a3D210C5eFC1A2Cb8FEEaBaD7Cb9ca' ],
  '5': [],
  __length__: 6,
  major: '1',
  minor: '2',
  patch: '4',
  commit: '0x0000000000000000000000000000000000000004',
  pass: [ '0x945E4C7991a3D210C5eFC1A2Cb8FEEaBaD7Cb9ca' ],
  fail: [] }

MicaiahReid added a commit to MicaiahReid/web3.js that referenced this issue Jan 28, 2022
jdevcs added a commit that referenced this issue Feb 8, 2022
* Make JsonRpcPayload's `params` field optional

Currently jsonrpc.js uses `params: params || []` in the
`toPayload` function, so this type update makes the `params` field
optional to match.

* Fix JsonRpcResponse type

Update `id` to accept `string | number` - this now matches the
`isValidResponse` function in `jsonrpc.js`.

Update `error` to accept an object with optional `code`, `data`,
and non-optional `message` fields to more closely match the
[JSON RPC spec](https://www.jsonrpc.org/specification#error_object)
and the `ErrorResponse` function in `errors.js`.

* Remove errant spaces

* Add PR #443 to CHANGELOG

Co-authored-by: jdevcs <[email protected]>
spacesailor24 added a commit that referenced this issue Mar 13, 2022
* Fix jsonrpc payload and response types (#4743)

* Make JsonRpcPayload's `params` field optional

Currently jsonrpc.js uses `params: params || []` in the
`toPayload` function, so this type update makes the `params` field
optional to match.

* Fix JsonRpcResponse type

Update `id` to accept `string | number` - this now matches the
`isValidResponse` function in `jsonrpc.js`.

Update `error` to accept an object with optional `code`, `data`,
and non-optional `message` fields to more closely match the
[JSON RPC spec](https://www.jsonrpc.org/specification#error_object)
and the `ErrorResponse` function in `errors.js`.

* Remove errant spaces

* Add PR #443 to CHANGELOG

Co-authored-by: jdevcs <[email protected]>

* changelog update

Co-authored-by: Micaiah Reid <[email protected]>
Co-authored-by: Wyatt Barnes <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Addressing a bug
Projects
None yet
Development

No branches or pull requests

3 participants