feat(admin): getL1ToL2MessageByTxHash(l1DepositTxHash) #280

s4rv4d · 2024-11-25T19:45:06Z

Description
added getL1ToL2MessageByTxHash(l1DepositTxHash), essentially letting devs fetch L1ToL2Message via SourceHash

Metadata

adds extra functionality requested in supersim admin API (read) #204

…sitIndexer

jakim929 · 2024-11-25T19:50:17Z

opsimulator/deposits.go

@@ -36,7 +36,7 @@ type LogSubscriber interface {
 }

 // transforms Deposit event logs into DepositTx
-func SubscribeDepositTx(ctx context.Context, logSub LogSubscriber, depositContractAddr common.Address, ch chan<- *types.DepositTx) (ethereum.Subscription, error) {
+func SubscribeDepositTx(ctx context.Context, logSub LogSubscriber, depositContractAddr common.Address, ch chan<- *types.DepositTx, chLog chan<- *types.Log) (ethereum.Subscription, error) {


high level it would be cleaner to just pass a single channel here.

if you need other info from the depositTx like some of the log data, you can create a wrapper that includes the deposit tx along with the log

jakim929 · 2024-11-25T19:51:53Z

opsimulator/opsimulator.go

 		portalAddress := common.Address(opSim.Config().L2Config.L1Addresses.OptimismPortalProxy)
-		sub, err := SubscribeDepositTx(context.Background(), opSim.l1Chain.EthClient(), portalAddress, depositTxCh)
+		sub, err := SubscribeDepositTx(context.Background(), opSim.l1Chain.EthClient(), portalAddress, depositTxCh, l1DepositTxnLogCh)


similar to the indexer/relayer design for l2 to l2 messages, it will be more maintainable to

have the indexer have a start function and run separately from the opsimulator. the op simulator then "listens" to the indexer to get the stream of deposit tx

all of the setting new logs logic should happen inside the deposit indexer

@jakim929 just clarifying the new indexer will be subscribing to deposit tx and storing, while the opsim would just be used to start the L1ToL2Indexer service correct?

while indexer listens for new tranction a transaction event should also be sent from indexer to opsim to SendTransaction to done while the indexer store ref to the initial depTxn sub message

or another way could be both opsim and indexer can both sub to deposit txn, the op sim executes transaction while the indexer just stores ref?

jakim929 · 2024-11-25T19:55:41Z

opsimulator/store.go

+)
+
+type L1DepositStore struct {
+	entryByHash map[common.Hash]*types.DepositTx


some high level thoughts

entryByHash should be indexed by something that globally & uniquely identifies a depositTX. the l1 txhash is insufficient because

you should prob maybe store more than just the deposit tx here since the indexer likely wants more info than that (ie. the log where this was emitted for instance)

update:

using the source hash as the key value, since it uniquely identifies the source of the deposit

updated the message to contain both the depositTxn and the initiating log in the form of a struct

jakim929 · 2024-11-25T19:56:50Z

opsimulator/opsimulator.go

@@ -168,6 +174,9 @@ func (opSim *OpSimulator) startBackgroundTasks() {
 				}

 				opSim.log.Info("OptimismPortal#depositTransaction", "l2TxHash", depTx.Hash().String())
+				if err := opSim.l1DepositStoreManager.Set(l1Log.TxHash, dep); err != nil {


a single l1 transaction can have multiple deposit tx inside it so it cannot be used as the index. instead the deposit tx's own transaction hash might be a better one

got it makes sense updating this

…sitIndexer

s4rv4d · 2024-12-12T13:20:36Z

sharing a excalidraw explanation on how it works

s4rv4d · 2024-12-12T13:27:01Z

HLD (high level design):

each opsim will listen to its own l1ToL2Indexer for deposit events via the deposit contract via a sub key ("DepositMessageKey:destination:chainId")
each indexer will filter the events for the specific deposit contract associated to each l2chain
once an event comes in and it is a deposit event, we process the event
processing contains to steps:

storing the event in via the store manager with source hash as key - since its a unique identifer for a deposit
publish the event so that the linked opsim can create a transaction

s4rv4d · 2024-12-12T13:28:44Z

Responsibilities:

Indexer - store/update deposit txn in store
Opsim - listen to indexer and execute deposit txn

jakim929 · 2024-12-23T18:08:24Z

Sorry for the delay here. This is looking pretty good!

I really like how opsim can subscribe to a subset of all events using the chainId using SubscribeDepositMessage.

A few suggestions for the high level architecture

A single instance of the indexer ideally should handle all chains (not 1 chain per indexer).
The indexer should be initialized at the orchestrator for ALL chains.
The indexer instance is passed to the opsim, where it listens to events for a specific chain.

I think both the admin endpoint and the opsim should query the indexer (not the store manager).

This provides better separation of responsibility because the only one that is in charge of managing the lifecycle of the indexer, and opsim / admin only use it on a readonly basis. This is similar to the approach for the l2 to l2 indexer.

Will leave comments on specific lines for this!

jakim929

Left a few high level comments about the design, please let me know if they require clarification!

jakim929 · 2024-12-23T18:08:39Z

opsimulator/opsimulator.go

+	if err := opSim.indexer.Start(ctx, opSim.l1Chain.EthClient(), opSim.Chain); err != nil {
+		return fmt.Errorf("L1ToL2Indexer failed to start: %w", err)
+	}


Orchestrator should start the indexer so remove logic here

jakim929 · 2024-12-23T18:09:53Z

opsimulator/indexer.go

+	}
+}
+
+func (i *L1ToL2MessageIndexer) Start(ctx context.Context, client *ethclient.Client, l2Chain config.Chain) error {


should accept an array of l2 chains - maybe this method can be come a private helper method startForChain?

and the new Start can loop through the array of chains and call this. Just a suggestion - many ways to do this

jakim929 · 2024-12-23T18:14:23Z

opsimulator/indexer.go

+
+	i.eb.Publish(depositMessageInfoKey(chainID), depTx)
+	return nil
+}


We should add a test at the indexer level in indexer_test.go. it should confirm that sending a tx gets correctly picked up, and the subscribe methods work correctly.

jakim929 · 2024-12-23T18:19:56Z

opsimulator/deposits.go

+type DepositChannels struct {
+	DepositTxCh chan<- *types.DepositTx
+	LogCh       chan<- types.Log
+}


We should ideally just remove this file altogether, and in the indexer logic just subscribe to the log.

The indexer can subscribe to the log, then manually convert it to a deposit tx.

jakim929 · 2024-12-23T18:20:38Z

admin/admin.go

+		}
+	}
+
+	storeEntry, err := m.l1DepositStore.Get(*args)


This should query the indexer not the store directly.

s4rv4d added 3 commits November 20, 2024 17:58

L1DepositStore - wip

bf02dac

Merge branch 'main' of https://github.com/s4rv4d/supersim into L1Depo…

150babd

…sitIndexer

rebase fix

2b0db5e

s4rv4d requested a review from a team as a code owner November 25, 2024 19:45

s4rv4d marked this pull request as draft November 25, 2024 19:45

jakim929 reviewed Nov 25, 2024

View reviewed changes

s4rv4d added 7 commits December 11, 2024 17:39

added modifications

7191e01

fixes

823c9f5

Merge branch 'main' of https://github.com/s4rv4d/supersim into L1Depo…

bc365ae

…sitIndexer

conflict fix

1e3ff9f

fixed: indexer setup

655818e

final changes

f74c4f6

fixed test case and use source hash as identifier

11ab649

s4rv4d marked this pull request as ready for review December 12, 2024 10:01

s4rv4d added 4 commits December 12, 2024 15:32

removed unneeded type

8f9f264

fixed comment

ef265c2

created a channel wrapper, to pass txnData and log

235ab76

created deposit message to include more info while storing

c9c372e

s4rv4d added 3 commits December 12, 2024 19:04

small comment update

8befd29

type update

9d8cf8a

variable name update

10df06a

jakim929 requested changes Dec 23, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(admin): getL1ToL2MessageByTxHash(l1DepositTxHash) #280

feat(admin): getL1ToL2MessageByTxHash(l1DepositTxHash) #280

s4rv4d commented Nov 25, 2024 •

edited

Loading

jakim929 Nov 25, 2024

jakim929 Nov 25, 2024 •

edited

Loading

s4rv4d Dec 5, 2024

s4rv4d Dec 5, 2024

s4rv4d Dec 5, 2024

jakim929 Nov 25, 2024

s4rv4d Dec 12, 2024

s4rv4d Dec 12, 2024

jakim929 Nov 25, 2024

s4rv4d Dec 5, 2024

s4rv4d commented Dec 12, 2024

s4rv4d commented Dec 12, 2024 •

edited

Loading

s4rv4d commented Dec 12, 2024

jakim929 commented Dec 23, 2024 •

edited

Loading

jakim929 left a comment

jakim929 Dec 23, 2024

jakim929 Dec 23, 2024

jakim929 Dec 23, 2024

jakim929 Dec 23, 2024

jakim929 Dec 23, 2024

feat(admin): getL1ToL2MessageByTxHash(l1DepositTxHash) #280

Are you sure you want to change the base?

feat(admin): getL1ToL2MessageByTxHash(l1DepositTxHash) #280

Conversation

s4rv4d commented Nov 25, 2024 • edited Loading

Choose a reason for hiding this comment

jakim929 Nov 25, 2024 • 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

s4rv4d commented Dec 12, 2024

s4rv4d commented Dec 12, 2024 • edited Loading

s4rv4d commented Dec 12, 2024

jakim929 commented Dec 23, 2024 • edited Loading

jakim929 left a 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

s4rv4d commented Nov 25, 2024 •

edited

Loading

jakim929 Nov 25, 2024 •

edited

Loading

s4rv4d commented Dec 12, 2024 •

edited

Loading

jakim929 commented Dec 23, 2024 •

edited

Loading