Skip to content
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

Disable access to pending txs from filters + subscriptions #118

Conversation

mdehoog
Copy link
Contributor

@mdehoog mdehoog commented Aug 22, 2023

Description
Currently op-stack chains can leak tx pool contents on shared nodes using two methods:

  • HTTP call to eth_newPendingTransactionFilter and subsequent calls to eth_getFilterChanges
  • WS eth_subscribe to newPendingTransactions

This PR disables these two methods by default (returning an error: pending tx filters are disabled). It also adds a new flag: --rollup.allowpendingtxfilters which optionally enables them.

Tests
Tested running geth locally. Without the option:

> curl -d '{"id":0,"jsonrpc":"2.0","method":"eth_newPendingTransactionFilter","params":[]}' -H "Content-Type: application/json" http://localhost:8545
{"jsonrpc":"2.0","id":0,"error":{"code":-32000,"message":"pending tx filters are disabled"}}

> wscat --connect ws://localhost:8546
> { "id": 1, "jsonrpc": "2.0", "method": "eth_subscribe", "params": ["newPendingTransactions"] }
< {"jsonrpc":"2.0","id":1,"error":{"code":-32000,"message":"pending tx filters are disabled"}}

With the --rollup.allowpendingtxfilters option:

> curl -d '{"id":0,"jsonrpc":"2.0","method":"eth_newPendingTransactionFilter","params":[]}' -H "Content-Type: application/json" http://localhost:8545
{"jsonrpc":"2.0","id":0,"result":"0x3fe165e46f7aa71c04f3e538839a029"}

> wscat --connect ws://localhost:8546
> { "id": 1, "jsonrpc": "2.0", "method": "eth_subscribe", "params": ["newPendingTransactions"] }
< {"jsonrpc":"2.0","id":1,"result":"0xf6f39af0ac3984a166ab78bc23a9aa9a"}

Additional context

https://status.base.org/incidents/m9fnx4p3bhp5

@mdehoog mdehoog force-pushed the disable-pending-txpool-access branch from 67352ce to 8e2ec3c Compare August 22, 2023 06:28
@mdehoog mdehoog force-pushed the disable-pending-txpool-access branch from 8e2ec3c to bff8b7b Compare August 22, 2023 17:14
Copy link
Contributor

@trianglesphere trianglesphere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I missed that tests need to be updated. Changes still look good though.

eth/filters/filter_system_test.go:266:10: assignment mismatch: 1 variable but api.NewPendingTransactionFilter returns 2 values
eth/filters/filter_system_test.go:323:10: assignment mismatch: 1 variable but api.NewPendingTransactionFilter returns 2 values
eth/filters/filter_system_test.go:918:10: assignment mismatch: 1 variable but api.NewPendingTransactionFilter returns 2 values (typecheck)

eth/filters/api.go Show resolved Hide resolved
@trianglesphere trianglesphere merged commit 88c9fa3 into ethereum-optimism:optimism Aug 22, 2023
@mdehoog mdehoog deleted the disable-pending-txpool-access branch August 22, 2023 18:23
mdehoog added a commit to mdehoog/op-geth that referenced this pull request Sep 1, 2023
…-pending-txpool-access"

This reverts commit 88c9fa3, reversing
changes made to bb04c5f.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants