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 listeners to cw4-group (and cw4?) #140

Closed
3 tasks done
ethanfrey opened this issue Nov 13, 2020 · 2 comments · Fixed by #156
Closed
3 tasks done

Add listeners to cw4-group (and cw4?) #140

ethanfrey opened this issue Nov 13, 2020 · 2 comments · Fixed by #156
Labels
multisig Related to a multisig epic

Comments

@ethanfrey
Copy link
Member

ethanfrey commented Nov 13, 2020

We should allow multiple contracts to be informed of changing group membership. This is a key feature so we can remove the restriction of immutible groups from #80

  • Define a message format the listeners receive (a diff of changes to weights)
  • Define a message to "register a listener". Note that if I register a listener that always errors, it will prevent you from ever updating the group - only the group owner should be able to add a listener (same as change members)
  • After UpdateMembers is executed, all registered listeners will get sent a message of this update in the same transaction.

Please code and test this in cw4-group, and update the cw4 spec to include this (both the types and the README)

Note, in some cases like #141 we may want to know both before and after weight for the changes, not just the new state. Consider how much overhead that information adds (do we need to make more queries? or do we make those already?) and what benefit it provides. You do not need to implement it, but please address this in the comments of the PR

@ethanfrey ethanfrey added the multisig Related to a multisig epic label Nov 13, 2020
@ethanfrey ethanfrey added this to To do in Contract development via automation Nov 13, 2020
@ethanfrey
Copy link
Member Author

@alpe I would like your quick review on this. Not about rust, but about handling listeners (errors or out of gas), which we have discussed in the frame of cron jobs.

Here we propose doing this atomically, so the UpdateMembers pays that gas and will not complete if the listener blocks it (I guess these could be called "hooks" not listeners). I require only the contract admin to be able to set those listeners, so I guess this is safe.

However, maybe there is another architecture that is more like listeners that we could imagine. That we write all diffs to a local queue in the contract (no DoS possible here). And "somehow" the listener contracts are triggered to query that queue and take any action they want - each listener being run in separate tx, and separate from the original tx.

If we have some way of isolating cron jobs / delayed tasks, and the called contract paying the gas to cover it, maybe the group can just request a future task for each listener, and these are run within that isolation, gas paid for my the listener. If we could actually design such a system in more than the "hand wavy" description I propose above, then this opens a lot of doors. Any contract could permissionlessly register as a listener to the group, as that action cannot effect the contract's execution at all.

I try to define "hooks" (run atomically with the action, can abort it) from "listeners" (run asynchronously after the action is committed, cannot make changes). Hooks are obviously the easiest and what I will build in this PR, but after our discussions on cron service today, I am curious what you think about the listeners concept (and if you can define it better than I)

@alpe
Copy link
Member

alpe commented Dec 4, 2020

@ethanfrey to separate hooks and listeners as separate concepts in designs is a good idea. I believe there are valid use cases for both types. The listeners are the more complex ones.
As pointed out we need to consider: gas limits, non atomic transaction behaviour, error feedback, fees and some basic security model on the receiving side. When this contract can register itself or the listener contracts with a cron then this could provide a new trigger that allows the async execution. Fees and gas though need to be set by the cw4 group contract as this depends on the use case.
There is some risk to consider when a huge number of listeners registers for an event

This was referenced Dec 4, 2020
@ethanfrey ethanfrey moved this from To do to In progress in Contract development Dec 4, 2020
Contract development automation moved this from In progress to Done Dec 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
multisig Related to a multisig epic
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

2 participants