search

0xPolygonZero / plonky2

Apache License 2.0
758 stars 281 forks source link

Change context to current context for BN precompiles #1428

Closed wborgeaud closed 9 months ago

wborgeaud commented 9 months ago

Storing data at context=0 for every call to the BN multiplication precompile causes bugs when the precompile is called more than once. For example, this tx (a zkSync proof verification) fails. Fixed by storing data in the current context instead.

sonarcloud[bot] commented 9 months ago

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

Nashtare commented 9 months ago

Side note @wborgeaud, shouldn't we need to do similar changes for SEGMENT_KERNEL_ECDSA_TABLE and SEGMENT_KERNEL_BN_PAIRING?

wborgeaud commented 9 months ago

Side note @wborgeaud, shouldn't we need to do similar changes for SEGMENT_KERNEL_ECDSA_TABLE and SEGMENT_KERNEL_BN_PAIRING?

Hm I think we should be safe with the ECDSA code, not sure about the pairing since it's also called multiple times in some txns. But I guess it doesn't hurt to change the context to be on the safe side. Will address this in a separate PR.