Skip to content
This repository has been archived by the owner on Oct 8, 2023. It is now read-only.

obront - LES transactions are not sent to the sequencer #101

Closed
sherlock-admin opened this issue Apr 7, 2023 · 2 comments
Closed

obront - LES transactions are not sent to the sequencer #101

sherlock-admin opened this issue Apr 7, 2023 · 2 comments
Labels
Non-Reward This issue will not receive a payout Sponsor Disputed The sponsor disputed this issue's validity

Comments

@sherlock-admin
Copy link
Contributor

obront

medium

LES transactions are not sent to the sequencer

Summary

LES (Light Ethereum Subprotocol) doesn't forward the transaction to the sequencer when receiving it over RPC.

Vulnerability Detail

When a user submits a transaction to op-geth node (validator/verifier mode), the node sends the transaction to the sequencer, which adds it to the tx pool.

func (b *EthAPIBackend) SendTx(ctx context.Context, tx *types.Transaction) error {
	if b.eth.seqRPCService != nil {
		data, err := tx.MarshalBinary()
		if err != nil {
			return err
		}
		if err := b.eth.seqRPCService.CallContext(ctx, nil, "eth_sendRawTransaction", hexutil.Encode(data)); err != nil {
			return err
		}
	}
	return b.eth.txPool.AddLocal(tx)
}

However, in LES mode, It only adds the transaction to the tx pool.

func (b *LesApiBackend) SendTx(ctx context.Context, signedTx *types.Transaction) error {
	return b.eth.txPool.Add(ctx, signedTx)
}

Impact

  • Transction isn't sent to the sequencer and will never be processed (submitted to L1).
  • Inconsistency among op-geth nodes validators/verifiers and the sequencer.
  • Additionally, from UX perspective, it is misleading as the user would think the transaction was submitted "successfully".

Code Snippet

https://github.com/ethereum-optimism/op-geth/blob/optimism-history/les/api_backend.go#L193-L195
https://github.com/ethereum-optimism/op-geth/blob/optimism-history/eth/api_backend.go#L253-L264

Tool used

Manual Review

Recommendation

Match this RPC change in the LES RPC.

Ref: https://op-geth.optimism.io/

Additional notes

This finding is inspired by issue #175 in the previous contest. It exists in the repository defined in-scope for the contest and the judge's comment justifying medium severity applies.

This was an oversight on Optimism's part and there are markers that would suggest it should be in scope.

Furthermore, it definitely seems like Optimism wishes to support LES mode, as the les directory has been updated 3 weeks ago.

@github-actions github-actions bot added Medium A valid Medium severity issue Has Duplicates A valid issue with 1+ other issues describing the same vulnerability labels Apr 10, 2023
@hrishibhat
Copy link
Contributor

Sponsor comment:
Bedrock does not support LES.

@hrishibhat hrishibhat added the Sponsor Disputed The sponsor disputed this issue's validity label Apr 16, 2023
@GalloDaSballo
Copy link
Collaborator

Siding with the sponsor since it was already marked as OOS in the previous contest

sherlock-audit/2023-01-optimism-judging#175

@sherlock-admin sherlock-admin added Non-Reward This issue will not receive a payout and removed Medium A valid Medium severity issue Has Duplicates A valid issue with 1+ other issues describing the same vulnerability labels Apr 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Non-Reward This issue will not receive a payout Sponsor Disputed The sponsor disputed this issue's validity
Projects
None yet
Development

No branches or pull requests

3 participants