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
Add missing Transfer and Approval events #266
Conversation
Added the events in the *Data methods, and in burn
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For this we have the ApprovalData and TransferData events, we can replace them for Transfer and Approval events instead of adding them
Don't we want to have an event that includes the data param? Using the original events only has the downside that they don't include that value |
Yes, but the data parameter can be extracted from the transaction data, I think that will be better because if not we will log more information than the one we need. Lets just use the ERC20 token events. |
Makes sense, I'll change it that way |
TransferData and ApprovalData are not needed, we can just use the existing Transfer and Approval. The data content can be fetched from the transactions if needed
@AugustoL: just made the changes, how does it look now? |
Great, thanks @nberger |
Added the events in the *Data methods, and in burn
Fixes #248