Realtime Subscription RPCs

[yaziciahmet]

Description

Initialized realtime RPCs. Currently only contains newHeads and logs subscriptions. I also introduced new config parameters: disable_subscriptions and max_subscription_connections.

Node implementations (sequencer, fullnode, prover) now receives a sender broadcast channel to notify about new L2 soft confirmations. RPC layer has its own wrapper around a receiver to this channel to actually use this notifications to query Ethereum blocks and send them through the Websocket connections.

In the future, when different types of notifications proves to be needed, it would make sense to replace this broadcast channel with ChainEvents struct which would handle different types of events happening on the chain.

Linked Issues

• Fixes Subscription based Ethereum RPCs #118
• Related to Websocket RPC #117

[yaziciahmet]

Q: Does this handle reorg out of the box? If not, what to do?

[yaziciahmet]

Q: Other than Ethereum block fields, this additionally returns l1FeeRate and l1Hash. Are we fine with that?

[codecov]

Codecov Report

Attention: Patch coverage is 87.23810% with 67 lines in your changes missing coverage. Please review.

Project coverage is 79.4%. Comparing base (f7d9207) to head (d141d6d).
Report is 2 commits behind head on nightly.

Additional details and impacted files

Files	Coverage Δ
bin/citrea/src/eth.rs	100.0% <100.0%> (ø)
bin/citrea/src/rollup/bitcoin.rs	0.0% <ø> (ø)
bin/citrea/src/rollup/mock.rs	95.0% <100.0%> (+0.1%)	⬆️
bin/citrea/src/test_rpc.rs	99.7% <100.0%> (+<0.1%)	⬆️
crates/ethereum-rpc/src/ethereum.rs	100.0% <100.0%> (ø)
crates/evm/src/query.rs	86.3% <100.0%> (-0.1%)	⬇️
crates/evm/src/rpc_helpers/filter.rs	68.7% <100.0%> (+10.7%)	⬆️
crates/evm/src/rpc_helpers/responses.rs	100.0% <100.0%> (ø)
crates/evm/src/smart_contracts/logs_contract.rs	84.3% <100.0%> (+7.1%)	⬆️
crates/fullnode/src/runner.rs	75.0% <100.0%> (+0.3%)	⬆️
... and 9 more

... and 3 files with indirect coverage changes

[eyusufatik]

Q: Does this handle reorg out of the box? If not, what to do?

I don't think so. But we don't reorg as of now. don't need to think about it

[eyusufatik]

Q: Other than Ethereum block fields, this additionally returns l1FeeRate and l1Hash. Are we fine with that?
yes we should

[eyusufatik]

good implementation. pls also think about @rakanalh 's comment on where the send on channel should happen.

I think we should have one more feature on this PR.

I don't want our rpc nodes to handle subscriptions. let's put a new boolean config on rollup config for RPC. you can call it allow_subscriptions or smth like that.

also there should be a max_subscriptions on jsonrpsee config, you can pass that through rollup config as well, check max_connections deafult can be something like 100 idk

[yaziciahmet]

After all the comments and discussions, here is how I see this PR going:

1. I add disable_subscriptions and max_subscription_count config parameters, which defaults to false and 100 respectively.
2. I create a EthereumSubscriptionManager type. Fullnode and Sequencer have an instance of this manager, and broadcasts l2 height when a new soft confirmation is committed using the manager. RPC methods also have access to this manager, and are able to subscribe to different types of events.
3. EthereumSubscriptionManager, based on the subscription type, queries the evm storage and broadcasts it to all the subscribers. This way, each subscriber won't have query the same evm block or logs again and again. We will be able to query it once, and copy it in memory, and send it to all subscribers.

With this approach, we don't need cache whatsoever since we just query once and send to everyone.

[yaziciahmet]

IMO there are 2 places where performance can be improved:

1. In logs subscription, for each filter in the hash map, we call evm.eth_get_logs(filter). Even though each log has to be filtered repeatedly for each one of the filters, calling the eth_get_logs high level method causes the block, transactions, and receipts to be queried again and again for each filter. With the cost of potential duplication, and the dirtiness of exposing some internal methods from evm, we can query all the receipts and even all the logs just once at the beginning, and just apply filters onto them.
2. Implementation of Hash trait for Filter. Currently, it is serialized into json and the bytes are hashed. But json is definitely not the optimal data type for serialization and in terms of byte-size. We can use Borsh maybe, or to avoid serialization completely, we can use unsafe Rust and get the memory layout of the struct and use that as the hash value.

[eyusufatik]

In logs subscription, for each filter in the hash map, we call evm.eth_get_logs(filter). Even though each log has to be filtered repeatedly for each one of the filters, calling the eth_get_logs high level method causes the block, transactions, and receipts to be queried again and again for each filter. With the cost of potential duplication, and the dirtiness of exposing some internal methods from evm, we can query all the receipts and even all the logs just once at the beginning, and just apply filters onto them.

why not get all logs at once, then use get_logs_in_block_range (or make a function that iterates over vec of logs and applies the filter use that in get_logs_in_block_range as well.)

Implementation of Hash trait for Filter. Currently, it is serialized into json and the bytes are hashed. But json is definitely not the optimal data type for serialization and in terms of byte-size. We can use Borsh maybe, or to avoid serialization completely, we can use unsafe Rust and get the memory layout of the struct and use that as the hash value.

are we sure if the logs subscription uses the same filter with eth_getLogs? I saw it has a field called pub block_option: FilterBlockOption does having this make sense on the subscription? should we handle it?

• we can look into importing Filter type from alloy-rpc-types. I guess when we were implementing logs we had to copy paste it over to our repo.

[yaziciahmet]

are we sure if the logs subscription uses the same filter with eth_getLogs? I saw it has a field called pub block_option: FilterBlockOption does having this make sense on the subscription? should we handle it?

They also accept block number and filter by that. Even though it doesn't make much sense, I think it is the general behavior of eth nodes, including reth implementation.

[yaziciahmet]

I have 4 questions left for this PR:

1. Should we spawn senders in a separate thread to not block receiving of blocks? Because currently if for some reason notifying subscribers on the SubscriptionManager takes some time, it prevents receiving incoming blocks.
2. Should we count debug_traceChain as subscription? It is not a realtime subscription, but it is handled through subscription. Should we also disable it through the enable_subscription flag?
3. Should we just disable subscriptions in prover? Check my comment above about this: Realtime Subscription RPCs #916 (comment)
4. Do we implement config parameters disable_subscriptions and max_subscription_count in this PR?

[yaziciahmet]

1. I don't think its necessary as it is doing very light work, if it takes longer than producing a block we have a completely different problem we have to attend to. And also these broadcast channels are buffered hence there is a lower chance these messages will be lost.
2. I think we should count debug_traceChain as subscription, and disable it if specified in config. Simply because it is still a websocket connection, and jsonrpsee does not provide a way to close the subscription from the server side (a bit weird imo). Hence, if client fails to handle the subscription closing, there could be empty websocket connections lying around. Even though websocket connections have a default timeout, imo best to consider this as subscription.
3. I think we should have the capability of subscriptions on prover, but if we or whoever is gonna run it wants to disable subscriptions, they can do it through rollup config with disable_subscriptions parameter.
4. I already did.

[eyusufatik]

btw keep the subscriptions on prover. we run the prover for the short-mid term future. we just won't enable subscriptions.

doesn't matter that much.

[rakanalh]

Looks good with small nits