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

MolochV2 event spec feedback WIP #68

Open
wants to merge 6 commits into
base: pull-pattern
Choose a base branch
from
Open

MolochV2 event spec feedback WIP #68

wants to merge 6 commits into from

Conversation

daodesigner
Copy link
Contributor

@daodesigner daodesigner commented Feb 13, 2020

WIP

  • Write V2 Contract Events Spec for Ragekick, InternalTransfer, Deposit

  • Review V2 Contract Events Spec (@ameensol )

  • Actually emit new events

  • Write Tests

  • Tests Passing

V2 Contract Events

  • event Ragekick(address indexed memberAddress, uint256 lootToBurn);
  • event InternalTransfer(address indexed from, address indexed to, address indexed token, uint256 amount);
  • event Deposit(address indexed memberAddress, address token, uint256 amount);

V2 Contract Events Tests

  • event Ragekick(address indexed memberAddress, uint256 lootToBurn);
  • event InternalTransfer(address indexed from, address indexed to, address indexed token, uint256 amount);
  • event Deposit(address indexed memberAddress, address token, uint256 amount);

MolochV2 Events Spec Feedback

  • The Ragequit event in _ragequit emits msg.sender — which is wrong for ragekicks. This needs to be memberAddress (as has been pointed out by @sbetamc earlier), or maybe we would even want a separate Ragekick event?
    Added the Ragekick event, will fixed the Ragequit param.

  • In general, is the "outside world" fine with the getters and events we currently have? For example, internal transfers don't emit events at all.
    Added InternalTransfer and Deposit events for public visibility. (this adds significant overhead to some functions that fire multiple InternalTransfer events.

@pet3r-pan
Copy link
Member

pet3r-pan commented Mar 6, 2020

Ping @ameensol, close or merge?

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.

2 participants