feat: initial Starknet support #4895

xJonathanLEI · 2023-09-27T08:55:50Z

This PR brings initial support for Starknet into graph-node. The implementation comes from starknet-graph, and has been used in production by the zkLend team for around a year. That said, the support enabled by this PR is not production ready:

Trigger filter is not implemented, and graph-node would always consume the entire Firehose block stream and build all possible triggers. No benchmark has been done yet, but this should hurt performance pretty bad. (Edit: now implemented in feat(starknet): firehose trigger filter #5322)
Data source templates are not supported. This hasn't been tested yet, but technically graph-node would panic if someone deploys a subgraph with Starknet data source templates defined.
Substream support has not been added (will take inspiration from Filipe/substreams triggers #4887).

Nevertheless, this PR is submitted here first to kickstart the integration process. Let's bring graph-node's amazing indexing capabilities to Starknet!

xJonathanLEI · 2023-09-28T16:01:12Z

Just force pushed the PR branch to resolve breaking changes from #4887. The branch is now up to date again.

mangas · 2023-09-29T09:13:47Z

chain/starknet/src/adapter.rs

+// stream, and the complete stream is always consumed, regardless of what the subgraph is indexing.
+// This is very bad for indexing performance and must be implemented for it to be considered
+// production-ready.
+// TODO: implement trigger filter for much better performance


Probably not worth leaving these TODOs in here

mangas · 2023-09-29T09:18:13Z

chain/starknet/src/chain.rs

+
+pub struct TriggersAdapter;
+
+// impl Chain {


can you please remove the comment?

mangas · 2023-09-29T09:24:42Z

chain/starknet/src/chain.rs

+        let mut triggers: Vec<_> = shared_block
+            .transactions
+            .iter()
+            .flat_map(|transaction| -> Vec<StarknetTrigger> {


Could you please move the Arc::new(transaction) up so that the clone in every step is a clone of the Arc instead of a clone of the transaction?

mangas · 2023-09-29T09:25:31Z

chain/starknet/src/chain.rs

+        filter: &crate::adapter::TriggerFilter,
+    ) -> Result<BlockWithTriggers<Chain>, Error> {
+        // We can't do any filtering here as `TriggerFilter` is useless.
+        // TODO: implement trigger filtering once `TriggerFilter` actually does something.


I think this can also be removed

mangas · 2023-09-29T09:27:38Z

chain/starknet/src/chain.rs

+        panic!("Should never be called since not used by FirehoseBlockStream")
+    }
+
+    // Used for reprocessing blocks when creating a data source.


I think this comment is mislead, perhaps taken from runner.rs, where this is somewhat true but more widely this is used to transform the Block data into chain-specific triggers.

mangas · 2023-09-29T09:43:58Z

chain/starknet/src/data_source.rs

+    /// if event.fromAddr matches the source address. Note this only supports the default
+    /// Starknet behavior of one key per event.
+    fn handler_for_event(&self, event: &StarknetEventTrigger) -> Option<MappingEventHandler> {
+        let event_key = FieldElement::from_byte_slice_be(event.event.keys.first()?).ok()?;


if the event only matches against 1 key maybe we could either filter the rest out on ingress (when decoding)

I agree that this is not optimal but I'm not sure what's the best approach for this is. Event is managed by prost and generated from graph-as-to-rust. We can technically change the firehose-starknet protobuf format to only emit 1 element, but that seems to be sub-optimal too, as the network technically allows multiple keys.

An alternative would be to create a new struct that resembles Event, but instead of using a Vec for keys, it uses Option<> to capture only the first element if there's any. For all other fields we can just move from the old Event struct.

But then we're just changing from checking whether the first element exists, to whether the Option<> is Some. I'm not sure whether this is an improvement.

As far as I can tell the events are not actually passed down to WASM so maybe we could just drop them at the firehose level. Since The AscTransaction type only cares about type and hash then I think ideally we should avoid sending the unnecessary data

We don't need to hold the PR for that, just ensure we follow up on it

By dropping them at the firsthose level, do you mean that we simply never emit it in firehose-starknet in the first place? I always had this impression that a Firehose implementation should be as faithful to the actual network state as possible, due to the quote I read somewhere that "it should be technically feasible to reconstruct an entire archive node from Firehose data along". Dropping them from firehose-starknet seems to go against this principle.

Of course, I'm not an expert on this and would make changes as you see fit. Please let me know your final decision on this. Thanks!

Also I don't understand the "events are not sent to WASM" part. Do you mean "event keys"? Cuz I'm quite certain event data are indeed passed?

We don't need to hold the PR for that, just ensure we follow up on it

Just saw this. Got it. But if we need to change the Firehose representation now would be the best time lol. Once people start using it that becomes a breaking change.

You are correct, the trigger is indeed passed fully, I was looking for an implement ToAsc... for event and didn't see one. Since it's passed to WASM then I think it's fine to keep all the keys, you may match on one but want to use the rest in mappings so let's leave it as is.

Sure. Thanks for confirming!

mangas · 2023-09-29T09:46:08Z

chain/starknet/src/data_source.rs

+
+// Starknet data source templates are not supported at the moment. This should be implemented for
+// the Starknet support to be considered production ready.
+// TODO: support Starknet data source template


remove TODO comment

mangas · 2023-09-29T09:46:28Z

chain/starknet/src/data_source.rs

+
+// Starknet data source templates are not supported at the moment. This should be implemented for
+// the Starknet support to be considered production ready.
+// TODO: support Starknet data source template


remove todo comment

mangas · 2023-09-29T09:49:11Z

graph/src/blockchain/mod.rs

@@ -395,6 +395,9 @@ pub enum BlockchainKind {
    Cosmos,

    Substreams,
+
+    /// StarkNet chains


This is not needed

mangas · 2023-09-29T09:52:37Z

chain/starknet/Cargo.toml

+prost = { workspace = true }
+prost-types = { workspace = true }
+serde = "1.0"
+starknet-core = "0.6.0"


Could we get away with using starknet-ff and duplicating the util function? Would be great if we could minimize the amount of deps brought in

~~Since we're calculating hashes we will also need starknet-crypto. I will update this part too.~~

(Edit: we don't need starknet-crypto since it's Keccak, not Pedersen. So we only need sha3, which is already pulled in anyways.)

Updated with code copied from starknet-rs.

1. Removes unnecessary TODO comments 2. Replaces `starknet-core` with `starknet-ff` to minimize deps 3. Fixes expensive `transaction` clone 4. Other misc changes

feat: initial Starknet support

95e6f83

xJonathanLEI force-pushed the dev/starknet branch from 66e205c to 95e6f83 Compare September 28, 2023 16:00

mangas reviewed Sep 29, 2023

View reviewed changes

feat: resolve PR comments

7dbd2f0

1. Removes unnecessary TODO comments 2. Replaces `starknet-core` with `starknet-ff` to minimize deps 3. Fixes expensive `transaction` clone 4. Other misc changes

mangas approved these changes Sep 29, 2023

View reviewed changes

mangas merged commit d2a3219 into graphprotocol:master Sep 29, 2023
7 checks passed

xJonathanLEI deleted the dev/starknet branch September 29, 2023 12:46

This was referenced Nov 29, 2023

feat: Starknet data source validation with improvements #5030

Merged

feat(ts): add Starknet data types graphprotocol/graph-tooling#1523

Merged

xJonathanLEI mentioned this pull request Feb 10, 2024

feat(starknet): trigger filter #5201

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: initial Starknet support #4895

feat: initial Starknet support #4895

xJonathanLEI commented Sep 27, 2023 •

edited

Loading

xJonathanLEI commented Sep 28, 2023 •

edited

Loading

mangas Sep 29, 2023

xJonathanLEI Sep 29, 2023

mangas Sep 29, 2023

xJonathanLEI Sep 29, 2023

mangas Sep 29, 2023

xJonathanLEI Sep 29, 2023

mangas Sep 29, 2023

xJonathanLEI Sep 29, 2023

mangas Sep 29, 2023

xJonathanLEI Sep 29, 2023

mangas Sep 29, 2023

xJonathanLEI Sep 29, 2023

mangas Sep 29, 2023

mangas Sep 29, 2023 •

edited

Loading

xJonathanLEI Sep 29, 2023

xJonathanLEI Sep 29, 2023

xJonathanLEI Sep 29, 2023

mangas Sep 29, 2023

xJonathanLEI Sep 29, 2023

mangas Sep 29, 2023

xJonathanLEI Sep 29, 2023

mangas Sep 29, 2023

xJonathanLEI Sep 29, 2023

mangas Sep 29, 2023

xJonathanLEI Sep 29, 2023

mangas Sep 29, 2023

xJonathanLEI Sep 29, 2023 •

edited

Loading

xJonathanLEI Sep 29, 2023

feat: initial Starknet support #4895

feat: initial Starknet support #4895

Conversation

xJonathanLEI commented Sep 27, 2023 • edited Loading

xJonathanLEI commented Sep 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mangas Sep 29, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xJonathanLEI Sep 29, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xJonathanLEI commented Sep 27, 2023 •

edited

Loading

xJonathanLEI commented Sep 28, 2023 •

edited

Loading

mangas Sep 29, 2023 •

edited

Loading

xJonathanLEI Sep 29, 2023 •

edited

Loading