-
Notifications
You must be signed in to change notification settings - Fork 74
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(admin): getL1ToL2MessageByTxHash(l1DepositTxHash) #280
base: main
Are you sure you want to change the base?
Changes from 3 commits
bf02dac
150babd
2b0db5e
7191e01
823c9f5
bc365ae
1e3ff9f
655818e
f74c4f6
11ab649
8f9f264
ef265c2
235ab76
c9c372e
8befd29
9d8cf8a
10df06a
de9457c
b30e1dc
503f617
000f5c9
392ef75
b98faf9
21444e5
fb3db88
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
logCh := make(chan types.Log) | ||
filterQuery := ethereum.FilterQuery{Addresses: []common.Address{depositContractAddr}, Topics: [][]common.Hash{{derive.DepositEventABIHash}}} | ||
logSubscription, err := logSub.SubscribeFilterLogs(ctx, filterQuery, logCh) | ||
|
@@ -61,6 +61,7 @@ func SubscribeDepositTx(ctx context.Context, logSub LogSubscriber, depositContra | |
continue | ||
} | ||
ch <- dep | ||
chLog <- &log | ||
case err := <-logErrCh: | ||
errCh <- fmt.Errorf("log subscription error: %w", err) | ||
case <-ctx.Done(): | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -58,10 +58,12 @@ type OpSimulator struct { | |
ethClient *ethclient.Client | ||
|
||
stopped atomic.Bool | ||
|
||
l1DepositStoreManager *L1DepositStoreManager | ||
} | ||
|
||
// OpSimulator wraps around the l2 chain. By embedding `Chain`, it also implements the same inteface | ||
func New(log log.Logger, closeApp context.CancelCauseFunc, port uint64, host string, l1Chain, l2Chain config.Chain, peers map[uint64]config.Chain, interopDelay uint64) *OpSimulator { | ||
func New(log log.Logger, closeApp context.CancelCauseFunc, port uint64, host string, l1Chain, l2Chain config.Chain, peers map[uint64]config.Chain, interopDelay uint64, depositStoreManager *L1DepositStoreManager) *OpSimulator { | ||
bgTasksCtx, bgTasksCancel := context.WithCancel(context.Background()) | ||
|
||
crossL2Inbox, err := bindings.NewCrossL2Inbox(predeploys.CrossL2InboxAddr, l2Chain.EthClient()) | ||
|
@@ -88,6 +90,8 @@ func New(log log.Logger, closeApp context.CancelCauseFunc, port uint64, host str | |
}, | ||
}, | ||
|
||
l1DepositStoreManager: depositStoreManager, | ||
|
||
peers: peers, | ||
} | ||
} | ||
|
@@ -148,8 +152,9 @@ func (opSim *OpSimulator) startBackgroundTasks() { | |
// Relay deposit tx from L1 to L2 | ||
opSim.bgTasks.Go(func() error { | ||
depositTxCh := make(chan *types.DepositTx) | ||
l1DepositTxnLogCh := make(chan *types.Log) | ||
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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @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? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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? |
||
if err != nil { | ||
return fmt.Errorf("failed to subscribe to deposit tx: %w", err) | ||
} | ||
|
@@ -160,6 +165,7 @@ func (opSim *OpSimulator) startBackgroundTasks() { | |
select { | ||
case dep := <-depositTxCh: | ||
depTx := types.NewTx(dep) | ||
l1Log := <-l1DepositTxnLogCh | ||
opSim.log.Debug("observed deposit event on L1", "hash", depTx.Hash().String()) | ||
|
||
clnt := opSim.Chain.EthClient() | ||
|
@@ -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 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. got it makes sense updating this |
||
opSim.log.Error("failed to store deposit tx to chain: %w", "chain.id", chainId, "err", err) | ||
} | ||
|
||
case <-opSim.bgTasksCtx.Done(): | ||
sub.Unsubscribe() | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,63 @@ | ||
package opsimulator | ||
|
||
import ( | ||
"fmt" | ||
"sync" | ||
|
||
"github.com/ethereum/go-ethereum/common" | ||
"github.com/ethereum/go-ethereum/core/types" | ||
) | ||
|
||
type L1DepositStore struct { | ||
entryByHash map[common.Hash]*types.DepositTx | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. some high level thoughts
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. update:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
mu sync.RWMutex | ||
} | ||
|
||
type L1DepositStoreManager struct { | ||
store *L1DepositStore | ||
} | ||
|
||
func NewL1DepositStore() *L1DepositStore { | ||
return &L1DepositStore{ | ||
entryByHash: make(map[common.Hash]*types.DepositTx), | ||
} | ||
} | ||
|
||
func NewL1DepositStoreManager() *L1DepositStoreManager { | ||
return &L1DepositStoreManager{ | ||
store: NewL1DepositStore(), | ||
} | ||
} | ||
|
||
func (s *L1DepositStore) Set(txnHash common.Hash, entry *types.DepositTx) error { | ||
s.mu.Lock() | ||
defer s.mu.Unlock() | ||
|
||
s.entryByHash[txnHash] = entry | ||
return nil | ||
} | ||
|
||
func (s *L1DepositStore) Get(txnHash common.Hash) (*types.DepositTx, error) { | ||
s.mu.RLock() | ||
defer s.mu.RUnlock() | ||
|
||
entry, exists := s.entryByHash[txnHash] | ||
|
||
if !exists { | ||
return nil, fmt.Errorf("Deposit txn not found") | ||
} | ||
|
||
return entry, nil | ||
} | ||
|
||
func (s *L1DepositStoreManager) Get(txnHash common.Hash) (*types.DepositTx, error) { | ||
return s.store.Get(txnHash) | ||
} | ||
|
||
func (s *L1DepositStoreManager) Set(txnHash common.Hash, entry *types.DepositTx) error { | ||
if err := s.store.Set(txnHash, entry); err != nil { | ||
return fmt.Errorf("failed to store message: %w", err) | ||
} | ||
|
||
return nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
package opsimulator | ||
|
||
import ( | ||
"math/rand" | ||
"testing" | ||
|
||
optestutils "github.com/ethereum-optimism/optimism/op-service/testutils" | ||
"github.com/ethereum/go-ethereum/common" | ||
"github.com/ethereum/go-ethereum/core/types" | ||
"github.com/stretchr/testify/assert" | ||
) | ||
|
||
func TestL1DepositStore_SetAndGet(t *testing.T) { | ||
sm := NewL1DepositStoreManager() | ||
|
||
rng := rand.New(rand.NewSource(int64(0))) | ||
sourceHash := common.Hash{} | ||
depInput := optestutils.GenerateDeposit(sourceHash, rng) | ||
depTx := types.NewTx(depInput) | ||
txnHash := depTx.Hash() | ||
|
||
err := sm.store.Set(txnHash, depInput) | ||
assert.NoError(t, err, "expect no error while store deposit txn ref") | ||
|
||
retrievedEntry, err := sm.store.Get(txnHash) | ||
assert.NoError(t, err, "expected no error when getting entry from store") | ||
assert.Equal(t, depInput, retrievedEntry, "expected retrieved entry to equal stored entry") | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should query the indexer not the store directly.