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

Improving of the EventLog interface #2436

Closed
nivida opened this issue Mar 1, 2019 · 6 comments
Closed

Improving of the EventLog interface #2436

nivida opened this issue Mar 1, 2019 · 6 comments
Labels
2.x 2.0 related issues Discussion Enhancement Includes improvements or optimizations Stale Has not received enough activity

Comments

@nivida
Copy link
Contributor

nivida commented Mar 1, 2019

Improving of the EventLog interface

Currently the indexed and the non-indexed decoded returned input parameters are returned within the returnValues property of the EventLog object. This property does contain an object which contains the returned topics and data properties encoded and added by the input name and input index.

The returnValues property does exist in a returned transaction receipt of a Contract method call or when you subscribe to a Contract event.

decodeLog(inputs: Array, data: string, topics: Array)

Feel free to write a comment with your opinion about it.

Expected behavior

{
  outputs: {
    namedParameter: 'value',
    nonIndexedNamedParameter: 'value'
  }
}

Actual behavior

{
  returnValues: {
    namedParameter: 'value',
    0: 'value',
    nonIndexedNamedParameter: 'value',
    1: 'value'
  }
}

Steps to reproduce the behavior

  1. Subscribe to a contract event

Versions

Web3.js latest version

@nivida nivida added Enhancement Includes improvements or optimizations Discussion labels Mar 1, 2019
@frozeman
Copy link
Contributor

frozeman commented Mar 1, 2019

The reason its called like returnValues is because its part fo the event which has many more properties.
inputs is a bad name as its unintuitive, it is the output of the event, but called inputs in the ABI. Depends fro what direction youre looking at it. When writing the smart contract its the inputs of the Event, but when reading the log its the output :)

Thats why i called it returnValues, to be clear and separate from that confusion.. The return value object is a class called ReturnValues i think (?, long time ago). It has also the numbered index in there. This is so to have more than one option to access named and non named parameters.

Its also important that this is the same output for all functions that return events or decoded function call outputs. They should all look the same.

@nivida
Copy link
Contributor Author

nivida commented Mar 1, 2019

@frozeman renamed it to outputs which I think is the most intuitive way.

Its also important that this is the same output for all functions that return events or decoded function call outputs. They should all look the same.

Yes, the decodeLog method will still exist.

@frozeman
Copy link
Contributor

frozeman commented Mar 1, 2019

The returnValues are in returned events, and when looking at the transactionReceipt and a few more places i think.

I think having the indexes as well there is important to be able to iterate and access unnamed parameters.
It basically behaves the same as decoded output from function calls.

@nivida
Copy link
Contributor Author

nivida commented Mar 1, 2019

Yes, the contract transaction receipts do have encoded logs. I'm decoding them with these two objects: https://github.com/ethereum/web3.js/tree/1.0/packages/web3-eth-contract/src/decoders

I think having them combined in one property is confusing. Having of named return values in an outputs property and adding an outputsByKey property with an array is probably also confusing.
This is why I think combining the decoded event input parameters in an Object with numbered properties isn't the most intuitive solution also when it's defined with two separate properties.

Another solution would be just to return an Array of inputs but I think this isn't intuitive.

@nivida
Copy link
Contributor Author

nivida commented Mar 1, 2019

Because the contained items in the returnValues are just properties of the event could we also call the property properties:

{
  properties: {from: "0x0000000000000000000000000000000000000000", to: "0x9CC9a2c777605Af16872E0997b3Aeb91d96D5D8c", tokens: "100000000000000000000000000"}
}

Edit:
The interface of the event log object should be written down in a similar way as a TS interface and added to the documentation.

@nivida nivida changed the title Simplification of the contract log decoding New EventLog Interface Mar 1, 2019
@nivida nivida changed the title New EventLog Interface Improving of the EventLog interface Mar 1, 2019
@nivida nivida added In Progress Currently being worked on and removed In Progress Currently being worked on labels Mar 1, 2019
@nivida nivida added the 2.x 2.0 related issues label Jun 20, 2019
@github-actions
Copy link

github-actions bot commented Jul 4, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions

@github-actions github-actions bot added the Stale Has not received enough activity label Jul 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.x 2.0 related issues Discussion Enhancement Includes improvements or optimizations Stale Has not received enough activity
Projects
None yet
Development

No branches or pull requests

2 participants