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

Add events for receiving and sending #133

Closed
ligi opened this issue Oct 8, 2019 · 4 comments · Fixed by #135
Closed

Add events for receiving and sending #133

ligi opened this issue Oct 8, 2019 · 4 comments · Fixed by #135

Comments

@ligi
Copy link

ligi commented Oct 8, 2019

As discussed with @rmeissner at the demo day before Devcon - opening this as an issue:

I understand that the events where not added to save gas-costs. But the costs are now just there in another form: the form of centralization and the need to run tracing servers. So I would love to open the discussion to change this. At least for receiving there should be an event thrown. IMHO the costs would not be significant and the benefits would outweigh the costs. Let me know what you think!

context:
https://gitter.im/gnosis/Safe?at=5d8c7a97290b8c354ad93891
coder5876/simple-multisig#41 (comment)

@rmeissner rmeissner added this to the contracts-safe-1.1.0 milestone Oct 15, 2019
@mpetrunic
Copy link

👍

@rmeissner
Copy link
Member

@ligi @mpetrunic could you comment on the PR what parameters you would like to see and what information should be indexed.

@mpetrunic
Copy link

Something like TxInitiated, TxConfirmed and TxExecuted with address of actor so we can setup notifications with some existing service like Tenderly when something happens. Instead of pinging team members and checking safe website every morning. I don't think I need anything indexed.

@rmeissner
Copy link
Member

See #135 for all the events.

@safe-global safe-global locked as resolved and limited conversation to collaborators Oct 15, 2019
rmeissner added a commit that referenced this issue Oct 28, 2019
Closes #133: Added events + required refactoring
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants