Sanitise eth68 announcement #6222

emhane · 2024-01-24T22:29:14Z

adds validation checks in tx fetcher for NewPooledTransactions68

empty announcement
invalid tx type, doesn't convert to type TxType
invalid tx size, zero or bigger than constant MAX_TX_SIZE
duplicate hash

crates/net/eth-wire/src/types/broadcast.rs

mattsse

This mixes rlp and validation.

this is a lot of added complexity based on size assumptions.

what's the benefit this gives us?

crates/net/eth-wire/src/types/broadcast.rs

mattsse · 2024-01-24T23:24:21Z

perhaps this would be more appropriate as a configurable setting inside the Transactionmanager/fetcher?

mattsse

this is very easy to follow.

now we need some good thresholds

crates/net/network/src/transactions/validation.rs

crates/primitives/src/transaction/tx_type.rs

Co-authored-by: Matthias Seitz <[email protected]>

…igmxyz/reth into emhane/sanitise-eth68-announcement

mattsse

only a few remaining nits

crates/net/network/src/peers/reputation.rs

crates/net/network/src/transactions/fetcher.rs

crates/net/network/src/transactions/validation.rs

crates/primitives/src/transaction/tx_type.rs

Co-authored-by: Matthias Seitz <[email protected]>

…igmxyz/reth into emhane/sanitise-eth68-announcement

…s are cached

crates/net/eth-wire/src/types/broadcast.rs

crates/net/network/src/transactions/mod.rs

crates/net/network/src/transactions/validation.rs

mattsse

I like this trait a lot!

last nit re upper bound, then send it

crates/net/network/src/transactions/validation.rs

mattsse

pending @Rjected an suggestion for removing entries

crates/net/network/src/transactions/validation.rs

Rjected

LGTM, pending the nitpick comments I left, I like the validation trait!

crates/net/network/src/peers/reputation.rs

crates/net/network/src/transactions/validation.rs

Co-authored-by: Dan Cline <[email protected]>

…igmxyz/reth into emhane/sanitise-eth68-announcement

emhane requested review from mattsse, Rjected and gakonst as code owners January 24, 2024 22:29

emhane commented Jan 24, 2024

View reviewed changes

crates/net/eth-wire/src/types/broadcast.rs Outdated Show resolved Hide resolved

Rjected reviewed Jan 24, 2024

View reviewed changes

crates/net/eth-wire/src/types/broadcast.rs Outdated Show resolved Hide resolved

DaniPopes reviewed Jan 24, 2024

View reviewed changes

crates/net/eth-wire/src/types/broadcast.rs Outdated Show resolved Hide resolved

crates/net/eth-wire/src/types/broadcast.rs Outdated Show resolved Hide resolved

DaniPopes reviewed Jan 24, 2024

View reviewed changes

crates/net/eth-wire/src/types/broadcast.rs Outdated Show resolved Hide resolved

mattsse reviewed Jan 24, 2024

View reviewed changes

crates/net/eth-wire/src/types/broadcast.rs Outdated Show resolved Hide resolved

crates/net/eth-wire/src/types/broadcast.rs Outdated Show resolved Hide resolved

emhane added 2 commits January 25, 2024 16:24

Driveby: Make sure cache doesn't fill with same peer

4a21d91

Validate eth68 announcements

ca622e8

emhane force-pushed the emhane/sanitise-eth68-announcement branch from f8fa2d3 to ca622e8 Compare January 25, 2024 17:35

emhane requested a review from mattsse January 25, 2024 17:37

mattsse requested changes Jan 25, 2024

View reviewed changes

emhane and others added 5 commits January 25, 2024 20:25

Plug validation into tx manager

d1e0b76

Mark LayerOne struct non-exhaustive

54db58e

Co-authored-by: Matthias Seitz <[email protected]>

Minimise list re-ordering

a5d9195

Merge branch 'emhane/sanitise-eth68-announcement' of github.com:parad…

06f359e

…igmxyz/reth into emhane/sanitise-eth68-announcement

fixup! Minimise list re-ordering

e9ec0bf

mattsse requested changes Jan 27, 2024

View reviewed changes

mattsse added the A-networking Related to networking in general label Jan 27, 2024

emhane and others added 9 commits January 27, 2024 18:47

Remove whitespace

e2921a9

Co-authored-by: Matthias Seitz <[email protected]>

Remove whitespace

b82c059

Co-authored-by: Matthias Seitz <[email protected]>

Remove unecessary check

58fb52a

Set placholder low value for reputation change on bad eth68 announcement

6ef18db

Drive-by: fix bug, bad transactions metric

e1734df

Fix bug, duplicate hashes not caught by spam filter

b78a977

Update comments

5882251

Fix lint

2cb3561

Merge branch 'emhane/sanitise-eth68-announcement' of github.com:parad…

4c154e7

…igmxyz/reth into emhane/sanitise-eth68-announcement

emhane added 6 commits January 29, 2024 18:58

Replace sort with hash map in duplicate detection

1211546

Merge branch 'emhane/sanitise-eth68-announcement' of github.com:parad…

3766e4d

…igmxyz/reth into emhane/sanitise-eth68-announcement

Fix test

ed06555

Debug test tx manager

f10c314

Add tests for eth66 validation

0f1bdb6

Fix test, hash map for sorting duplicates changes order of how entrie…

9d21851

…s are cached

emhane requested a review from mattsse January 29, 2024 20:47

mattsse requested changes Jan 30, 2024

View reviewed changes

Addres review and make tx type encoded lenght limits lax

f3447be

emhane commented Jan 30, 2024

View reviewed changes

crates/net/network/src/transactions/validation.rs Outdated Show resolved Hide resolved

mattsse requested changes Jan 30, 2024

View reviewed changes

crates/net/network/src/transactions/validation.rs Outdated Show resolved Hide resolved

crates/net/network/src/transactions/validation.rs Outdated Show resolved Hide resolved

crates/net/network/src/transactions/validation.rs Outdated Show resolved Hide resolved

Set reasonable max encoded tx length limit

44ee97c

mattsse approved these changes Jan 30, 2024

View reviewed changes

crates/net/network/src/transactions/validation.rs Outdated Show resolved Hide resolved

Rjected approved these changes Jan 30, 2024

View reviewed changes

emhane and others added 8 commits January 30, 2024 21:26

Fix tests and docs

3551671

Remove unnecessary op

d78ea3c

Fix docs

20c8493

Co-authored-by: Dan Cline <[email protected]>

Add docs for eth filter

85d212a

Add test for derive more's display impl for zst

203b4b1

Merge branch 'emhane/sanitise-eth68-announcement' of github.com:parad…

59ac1c2

…igmxyz/reth into emhane/sanitise-eth68-announcement

Fix lint

6dbe077

Merge branch 'main' into emhane/sanitise-eth68-announcement

2e5062d

emhane enabled auto-merge January 31, 2024 12:37

Fix lint

fd2db04

emhane added this pull request to the merge queue Jan 31, 2024

Merged via the queue into main with commit 34cda3a Jan 31, 2024
29 checks passed

emhane deleted the emhane/sanitise-eth68-announcement branch January 31, 2024 13:33

mattsse mentioned this pull request Feb 1, 2024

fix: ensure backwards compat config #6319

Merged

rswanson mentioned this pull request Feb 9, 2024

swanny/assertoor #6515

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sanitise eth68 announcement #6222

Sanitise eth68 announcement #6222

emhane commented Jan 24, 2024 •

edited

Loading

mattsse left a comment

mattsse commented Jan 24, 2024 •

edited

Loading

mattsse left a comment

mattsse left a comment

mattsse left a comment

mattsse left a comment

Rjected left a comment

Sanitise eth68 announcement #6222

Sanitise eth68 announcement #6222

Conversation

emhane commented Jan 24, 2024 • edited Loading

mattsse left a comment

Choose a reason for hiding this comment

mattsse commented Jan 24, 2024 • edited Loading

mattsse left a comment

Choose a reason for hiding this comment

mattsse left a comment

Choose a reason for hiding this comment

mattsse left a comment

Choose a reason for hiding this comment

mattsse left a comment

Choose a reason for hiding this comment

Rjected left a comment

Choose a reason for hiding this comment

emhane commented Jan 24, 2024 •

edited

Loading

mattsse commented Jan 24, 2024 •

edited

Loading