search

polkadot-evm / frontier

Ethereum compatibility layer for Substrate.
Apache License 2.0
574 stars 487 forks source link

bound VariadicValue multiple variant #1488

Open librelois opened 3 months ago

librelois commented 3 months ago

VariadicValue::Multiple variant should be bounded to prevent heavy computation on cartesian product

boundless-forest commented 3 months ago

Does that have a significant performance issue, or could you please provide a detailed example of the case?

I find that this code snippet was copied from the previous parity Ethereum, and I wonder why it did not have this restriction before.

librelois commented 3 months ago

@boundless-forest this change comes from an internal review of possible flaws in the code, and we're trying to bound all unbounded inputs. The question is, what's a reasonable limit?

I wonder why it did not have this restriction before.

parity ethereum hasn't been used in production for years, it's not shocking that this client had several flaws that were less annoying at a time when ethereum was much less popular and therefore less hacked. So I wouldn't rely on it.

boundless-forest commented 2 months ago

The question is, what's a reasonable limit?

Yes. This limit will affect two areas: filter addresses and topics. For topics, a value over 4 is sufficient. However, for addresses, there is no explicitly defined number in the Ethereum protocol.

Have you considered the default JSON-RPC payload size restriction? It might address your concern.