hicommonwealth / commonwealth

A platform for decentralized communities
https://commonwealth.im
GNU General Public License v3.0
66 stars 41 forks source link

Generate EVM notifications #4863

Closed timolegros closed 10 months ago

timolegros commented 1 year ago

Description

Alchemy Notify allows us to receive Webhook notifications for any on-chain event from Ethereum, Polygon, Optimism, and Arbitrum without having to run any of our own indexing services.

Edit: new spec

Engineering Requirements

Acceptance Criteria

Additional context

A future issue/upgrade (done in tandem with ABI integration) should create a route which maps an ABI event signature to an Alchemy GraphQL object such that we can pragmatically update the Alchemy webhook. For the moment, (given that the EVM chains with has_chain_events_listener hasn't changed in ages) updating the webhook for a new chain requires a new PR.

jnaviask commented 1 year ago

I am a little worried about vendor lock-in here + also limiting our support for notifications to Alchemy-supported EVM chains. I don't recall us landing on a decision here -- we may need to pause this ticket until we have a feature matrix built out, cc @dillchen @zakhap.

zakhap commented 11 months ago

Definitely do not want to have strict vendor lock-in here, as competing evm compatible chain ecosystems (like BNB, for example) may have their own data providers we could use for offering similar notification support. I know that Growth team is waiting patiently on the sidelines for Gating/Groups to go in so can begin to support the requests of quite a few L2 ecosystems that want to join Common so we should be flexible enough in our decision here to also work with them for Notifications too.

timolegros commented 11 months ago

Using Alchemy to support notifications for the major EVM chains doesn't explicitly mean we can't support notifications on other EVM chains (can be reviewed on a case-by-case basis). In other words, we can implement notifications for Ethereum + L2's for now and then add notifications for additional chains in the future based on the providers that are available for those chains. We should definitely have a conversation with Growth so that the eng effort it takes to build/maintain these notification features provides a reasonable amount of business value. No point in supporting notifications for a chain that has 2 users on CW and no new communities in the growth pipeline.

If we really don't want to use Alchemy Webhooks then I don't really see a way around creating a CE 2.0 i.e. a light version of chain events solely for EVM with generic block parsing using contract addresses and event signatures that doesn't populate any table other than the notifications table. Basically a less bloated version of chain-events with no contract types or cosmos support. Was trying to avoid this because it means we have to deal with all the same problems as before like uptime (playing catch-up after downtime), server orchestration (memory/load management), etc.

timolegros commented 11 months ago

After today's (Aug 31) protocol office hours with Zak it has become clear that we want to drive toward a future where users can subscribe to events from various EVM contracts on any EVM chain. Given this broad constraint, it is necessary to create CE 2.0. CE 2.0 will be an EVM-only, contract-type free, polling-based, single-dyno, queueless version of chain-events. It is meant to be extremely lightweight and modular. Supporting new event types/sources should require 0 modifications to CE 2.0.

Part 1 rough sketch:

Part 2 rough sketch: Also discussed, were the client-side changes required for users to be able to subscribe to event sources. This portion of changes should be considered a follow-up because it requires unlinking chain-event notifications from community chain_id which is a significant undertaking. This is to say, the first portion of the project will hardcode the event sources such that existing functionality is maintained and the follow-up will open up new functionality.

In order to construct community feeds from this setup, we can create a new 'community-feed' subscription category which means a feed is a set of subscriptions to event sources.

jnaviask commented 11 months ago

Couple of questions:

timolegros commented 11 months ago

Couple of questions:

  • @zakhap do we have clear product requirements written out anywhere for notifications? We discussed it on a call, but would like it memorialized so we have a reliable record of our goals.
  • @timolegros what would the architecture of CE 2.0 look like? Would it rely on the commonapp database or have its own? How would queuing and hosting work?

Single dyno worker in the Commonwealth App (like the CW Consumer). Uses the Commonwealth database directly. There is no queueing involved.

Probably the most important part that hasn't been defined yet is what the EVM chain-event notifications actually look like (what info they contain) and how we label them @zakhap. Currently, we hardcode labels for event types but this is not sustainable if we will support any event type on any chain. We either need to derive a name from the contract/ABI or ask the user to label the event source when subscribing. CE 2.0 should not (and won't) involve any labelling like CE 1.0.

That said, we can move forward with CE 2.0 without defining everything above if our goal is simply to maintain existing functionality (currently hardcoded labels can be moved to subscription records and applied during notification fetching). The scope of this issue should be to replace chain-events while maintaining existing functionality and not introducing new chain-event notification functionality client side.

zakhap commented 11 months ago

The scope of this issue should be to replace chain-events while maintaining existing functionality and not introducing new chain-event notification functionality client side.

I agree with this!

@zakhap do we have clear product requirements written out anywhere for notifications? We discussed it on a call, but would like it memorialized so we have a reliable record of our goals.

And will attend to this (and @timolegros specific questions) this week in the PRD

dillchen commented 11 months ago

First, the open / draft PRs look great so far and really opens the way for:

Templatized Chain Event Labels and admins to add their own ABIs as the spec and implementation make it easily generic, very exciting.

My question relates to #4769, and it's use of typed interfaces for fetching events / and specific readOnly events

As the goal of this ticket is to remove typed interfaces. Has any specific thought be given to further getting rid of use of type contracts for viewing?

This is out of scope to be clear, but I can draw up the requirements for this in a PRD if necessary, for lets call them [Templatized Chain Pages] -> which would rely on Templatized Chain Event Labels and CAT in general.

timolegros commented 11 months ago

Templatized chain pages might be doable but they are imo not practical or desirable from an engineering perspective. Labelling chain-events is one thing, but labeling functions and function return values is another. It introduces a lot of complexity and edge cases that are not easily foreseen. For example, Alpha proposals have a sequential proposal id starting at 0. Bravo proposals have a sequential id that starts at some admin-defined value. OZ-Bravo uses non-sequential hash ids. These are all implementation details that affect how we fetch proposals. If you wanted to avoid hardcoding data fetching schemes for ABIs you would need the user to define a fetching scheme (e.g. 'use X function to get ids and Y function to get proposal Rx[i]').

In other words, we would be asking users to define how to fetch data from a contract. Even if we used events and their labels to construct proposals like with CE 1.0 we still run into edge case issues for example, Aave has IPFS data which needs to be fetched to properly display a proposal (this action would need to be defined by a community admin).

Additionally, integrating a new governance scheme isn't really all that difficult (when using contract types) and the majority of projects re-use popular/existing contracts so this seems like a ton of effort for little real value.

@ianrowan am I missing something?

dillchen commented 11 months ago

Thanks for the comment. General agreement here. Good to memorialize. Esp cause I hadn't seen the trade offs made explicit.


TLDR view only functions formatted nicely probably not worth it atm.

Will post follow up comments here however

timolegros commented 10 months ago

Blocking #5247