-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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: api: sanity check the "to" address of outgoing messages #12135
Conversation
If the "to" address of an outgoing message is a _delegated_ address, verify that it maps to a valid Ethereum address. This isn't a consensus critical change, but it'll help prevent client-side address conversion libraries from directing messages into oblivion (e.g., by mis-translating `0xff0000....` addresses into `f410f...` addresses instead of `f0...` addresses.
d2285e0
to
3d1bc96
Compare
This seems fine to me, but here's my understanding of what it's doing: refusing to receive messages for the message pool from the API. One implication is that it would refuse such messages from the large public RPC providers, which would cut off a large % of the source of these messages (I think). I'm not sure how the pubsub censorship resistance works though so can't really comment on that. Is there any path to receiving and passing them on but just refusing to pack them into blocks? Or is the packing part of the protocol too? |
Added the backport label to this so we have it in the v1.28 release. |
Assigning myself to take this to completion. |
@Stebalien @rvagg I have added unit tests and an itest to this. Please can you guys approve? |
LGTM but I can't approve as I created the PR (and someone else should make sure the logic is fine). |
* feat: api: sanity check the "to" address of outgoing messages If the "to" address of an outgoing message is a _delegated_ address, verify that it maps to a valid Ethereum address. This isn't a consensus critical change, but it'll help prevent client-side address conversion libraries from directing messages into oblivion (e.g., by mis-translating `0xff0000....` addresses into `f410f...` addresses instead of `f0...` addresses. * tests for invalid delegated addresses * fix lint --------- Co-authored-by: aarshkshah1992 <[email protected]>
* feat: api: sanity check the "to" address of outgoing messages If the "to" address of an outgoing message is a _delegated_ address, verify that it maps to a valid Ethereum address. This isn't a consensus critical change, but it'll help prevent client-side address conversion libraries from directing messages into oblivion (e.g., by mis-translating `0xff0000....` addresses into `f410f...` addresses instead of `f0...` addresses. * tests for invalid delegated addresses * fix lint --------- Co-authored-by: aarshkshah1992 <[email protected]>
* feat: api: sanity check the "to" address of outgoing messages If the "to" address of an outgoing message is a _delegated_ address, verify that it maps to a valid Ethereum address. This isn't a consensus critical change, but it'll help prevent client-side address conversion libraries from directing messages into oblivion (e.g., by mis-translating `0xff0000....` addresses into `f410f...` addresses instead of `f0...` addresses. * tests for invalid delegated addresses * fix lint --------- Co-authored-by: aarshkshah1992 <[email protected]>
* feat: api: sanity check the "to" address of outgoing messages If the "to" address of an outgoing message is a _delegated_ address, verify that it maps to a valid Ethereum address. This isn't a consensus critical change, but it'll help prevent client-side address conversion libraries from directing messages into oblivion (e.g., by mis-translating `0xff0000....` addresses into `f410f...` addresses instead of `f0...` addresses. * tests for invalid delegated addresses * fix lint --------- Co-authored-by: aarshkshah1992 <[email protected]>
* feat: api: sanity check the "to" address of outgoing messages If the "to" address of an outgoing message is a _delegated_ address, verify that it maps to a valid Ethereum address. This isn't a consensus critical change, but it'll help prevent client-side address conversion libraries from directing messages into oblivion (e.g., by mis-translating `0xff0000....` addresses into `f410f...` addresses instead of `f0...` addresses. * tests for invalid delegated addresses * fix lint --------- Co-authored-by: aarshkshah1992 <[email protected]>
Related Issues
Proposed Changes
If the "to" address of an outgoing message is a delegated address, verify that it maps to a valid Ethereum address. This isn't a consensus critical change, but it'll help prevent client-side address conversion libraries from directing messages into oblivion (e.g., by mis-translating
0xff0000....
addresses intof410f...
addresses instead off0...
addresses.Notes:
Additional Info
Checklist
Before you mark the PR ready for review, please make sure that:
<PR type>: <area>: <change being made>
fix: mempool: Introduce a cache for valid signatures
PR type
: fix, feat, build, chore, ci, docs, perf, refactor, revert, style, testarea
, e.g. api, chain, state, mempool, multisig, networking, paych, proving, sealing, wallet, deps