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

Consider what contract event to implement (if any) #2265

Closed
autholykos opened this issue Sep 4, 2024 · 4 comments
Closed

Consider what contract event to implement (if any) #2265

autholykos opened this issue Sep 4, 2024 · 4 comments
Assignees
Labels
module:contracts Issues related to the genesis contracts module:execution-core Issues related to execution-core

Comments

@autholykos
Copy link
Member

Possible events are:

  • Moonlight Transfer
  • Addition to the note tree for Phoenix (emit the note or the nullifier) - this one we might seat out. To be discussed with @herr-seppia
  • Stake
@autholykos autholykos added module:execution-core Issues related to execution-core module:contracts Issues related to the genesis contracts labels Sep 4, 2024
@autholykos
Copy link
Member Author

autholykos commented Sep 4, 2024

After a brief discussion with @ureeves and @herr-seppia we recognized that, while Stake events are necessary, we need to carefully consider whether or not loading the contract execution with the overhead of emitting other type of events.

  • Provisioners do not have any use for events because their focus on achieving consensus requires limiting their interaction with clients as much as possible.
  • Archivals might serve different data to different clients. Therefore, it makes perfect sense to create events locally following a Moonlight or Phoenix transaction acceptance. These events might be tailor-made depending on the client requesting them. the convenience that an event would introduce for clients would still be possible with the Archival's intermediation.
  • Stake events are necessary because it would be quite difficult for an Archival to capture state changes triggered by transactions that are not initiated by a user, but are contract function calls performed by the consensus (e.g. slashing events).

Crafting events in the Archival rather than during contract execution introduces inefficiency in proving the correctness, consistency, and completeness of events. In fact, a client wishing to prove that the events sent by the Archival are honest, might have to extract events from the transactions, rather than collecting them directly from a transfer execution. Although event emission in contract is not free and introduces a (perhaps moderate) hike in gas price and overhead, this seems to be justified in the case of Moonlight transactions.

Paraphrasing @herr-seppia:

In my view, events should be generated exclusively by the VM. Everything else is up to the discretion of the archival implementation(s).
There should be a clear distinction between a genuine contract event and manipulated, unverifiable data.

@autholykos
Copy link
Member Author

Another reason that strengthens the case of having the contract emitting Moonlight events has to do with the fact that the Archival would already need to store the Stake events and those must necessarily be created during the contract execution. It would indeed be undesirable to have half of the events created by the VM and some others created by the Archival in an arbitrary fashion.

@ureeves
Copy link
Member

ureeves commented Sep 6, 2024

@herr-seppia and I have considered the events to emit as a matter of course for the protocol. Here's our notes from the meeting, and we will be proceeding to implement them all today.

Transfer Events

  • Mint
    • Add event with value and receiver of funds
  • Withdraw
    • Add event with value and receiver of funds
  • Convert
    • Add event with value and receiver of funds
    • Investigate whether the sender of the funds can be used (probably yes)
  • Deposit
    • Add event with value and contract receiving the funds
    • Investigate whether the sender of the funds can be used (probably not)
  • TransferToContract
    • Add event with the source and receiver contract, and value transferred
  • TransferToAccount
    • Add event with the source contract and receiver account, and value transferred
  • Spend and Execute
    • Add events on transfer of funds from account to account, and investigate during implementation if phoenix sender/receiver events also make sense.

Stake Events

  • Stake
    • Emit event with the account that staked and the staked amount
  • Unstake
    • Remove receiver of the withdraw, relying on the transfer contract to inform of the mint
  • Withdraw
    • Remove receiver of the withdraw, relying on the transfer contract to inform of the mint
  • Reward
  • Slash
    • Merge the suspended and slash events into one
  • Hard Slash
    • Merge the suspended and hard_slash events into one

ureeves pushed a commit that referenced this issue Sep 6, 2024
The following events are added, which are emitted when the function with
their same name is successful:

- `mint`
- `withdraw`
- `convert`
- `deposit`
- `transfer_to_contract`
- `transfer_to_account`

Additionally an event is emitted at the end of any transaction
ingestion. This event will either be `phoenix` or `moonlight`, depending
on the model used, and are emitted during the host's call to `refund`.
This is so that the event data can contain both the change and gas
spent.

As a consequence of the above changes, we needed to slightly change the
way in which we insert notes into the tree, and make some changes to the
`transitory` module to support including the change note in the emission
event.

See-also: #2265
@autholykos
Copy link
Member Author

Solved by #2295 and #2294

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module:contracts Issues related to the genesis contracts module:execution-core Issues related to execution-core
Projects
None yet
Development

No branches or pull requests

3 participants