Add Governance action event support #19252
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
3 Skipped Deployments
|
Bridgerz
force-pushed
the
bridge-indexer-governance
branch
from
dfed8db
to
ebf589f
Compare
Bridgerz
requested review from
dariorussi,
longbowlu and
patrickkuo
and removed request for
dariorussi and
longbowlu
| @@ -142,6 +142,56 @@ fn process_sui_event( | |||
| data: None, | |||
| })) | |||
| } | |||
| "UpdateRouteLimitEvent" => { | |||
Bridgerz
force-pushed
the
bridge-indexer-governance
branch
from
ebf589f
to
9b9f645
Compare
| } | ||
| EthSuiBridgeEvents::InitializedFilter(_) => {} |
There was a problem hiding this comment.
we need to capture these, and set alerts (they shouldn't happen though)
crates/sui-bridge-indexer/src/migrations/2024-09-05-180103_governance_actions/up.sql
Show resolved
Hide resolved
| } | ||
| EthBridgeConfigEvents::InitializedFilter(_) => {} | ||
| }, | ||
| EthBridgeEvent::EthCommitteeUpgradeableContractEvents(_) => {} |
There was a problem hiding this comment.
we don't care about this?
There was a problem hiding this comment.
no we're capturing a different upgrade event
There was a problem hiding this comment.
what happens when this event gets emitted? should we scream?
There was a problem hiding this comment.
I have added warn log for all the unexpected events.
| @@ -1,6 +1,8 @@ | |||
| CREATE TABLE governance_actions | |||
| ( | |||
| id BIGSERIAL PRIMARY KEY, | |||
| nonce BIGINT, | |||
| data_source TEXT NOT NULL, | |||
There was a problem hiding this comment.
we should use chain_id v.s. data_source for convention
There was a problem hiding this comment.
I was following how we are currently doing it for the "token transfer" table.
There was a problem hiding this comment.
chain id and datasource is 2 different thing, chain id is part of the key of the bridge message and the datasource in token transfer table is referring to the source of the data.
There was a problem hiding this comment.
in this case I think chain id is more appropriate, we might also want to include the message type as well.
There was a problem hiding this comment.
do you plan to want this to chain_id in this PR ? @patrickkuo
There was a problem hiding this comment.
we will need to include the chain id in the event to be able to record it, which require more changes, will need to leave it post launch
| txn_digest bytea NOT NULL, | ||
| sender_address bytea NOT NULL, | ||
| timestamp_ms BIGINT NOT NULL, | ||
| action text NOT NULL, |
There was a problem hiding this comment.
same, avoid using string, there is BridgeActionType https://github.com/MystenLabs/sui/blob/main/crates/sui-bridge/src/types.rs#L197
There was a problem hiding this comment.
Are you suggesting we update the db schema to include a "BridgeActionType" enum? We aren't doing this with any of our other enums (for instance status and data_source are also saved as Text), but not a bad idea...
There was a problem hiding this comment.
yeah that's the suggestion. I think we should rework all of them to integer post launch
patrickkuo
force-pushed
the
bridge-indexer-governance
branch
from
f154589
to
593f479
Compare
| @@ -1,6 +1,8 @@ | |||
| CREATE TABLE governance_actions | |||
| ( | |||
| id BIGSERIAL PRIMARY KEY, | |||
| nonce BIGINT, | |||
| data_source TEXT NOT NULL, | |||
There was a problem hiding this comment.
do you plan to want this to chain_id in this PR ? @patrickkuo
Description
Governance action support
Test plan
Spot check on my local machine. Indexer validity checker tool coming soon...