chainwayxyz / citrea

Citrea, Bitcoin's First ZK Rollup 🍊🍋
https://citrea.xyz
GNU General Public License v3.0
114 stars 22 forks source link

Backport lru deadlock fix #826

Closed kpp closed 2 months ago

kpp commented 3 months ago

Description

Linked Issues

codecov[bot] commented 3 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 78.2%. Comparing base (b3fb9fc) to head (1ee35a1).

Additional details and impacted files | [Files](https://app.codecov.io/gh/chainwayxyz/citrea/pull/826?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=chainwayxyz) | Coverage Δ | | |---|---|---| | [crates/ethereum-rpc/src/gas\_price/cache.rs](https://app.codecov.io/gh/chainwayxyz/citrea/pull/826?src=pr&el=tree&filepath=crates%2Fethereum-rpc%2Fsrc%2Fgas_price%2Fcache.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=chainwayxyz#diff-Y3JhdGVzL2V0aGVyZXVtLXJwYy9zcmMvZ2FzX3ByaWNlL2NhY2hlLnJz) | `95.1% <100.0%> (-0.3%)` | :arrow_down: | | [crates/ethereum-rpc/src/gas\_price/fee\_history.rs](https://app.codecov.io/gh/chainwayxyz/citrea/pull/826?src=pr&el=tree&filepath=crates%2Fethereum-rpc%2Fsrc%2Fgas_price%2Ffee_history.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=chainwayxyz#diff-Y3JhdGVzL2V0aGVyZXVtLXJwYy9zcmMvZ2FzX3ByaWNlL2ZlZV9oaXN0b3J5LnJz) | `98.6% <100.0%> (-0.1%)` | :arrow_down: | | [crates/ethereum-rpc/src/gas\_price/gas\_oracle.rs](https://app.codecov.io/gh/chainwayxyz/citrea/pull/826?src=pr&el=tree&filepath=crates%2Fethereum-rpc%2Fsrc%2Fgas_price%2Fgas_oracle.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=chainwayxyz#diff-Y3JhdGVzL2V0aGVyZXVtLXJwYy9zcmMvZ2FzX3ByaWNlL2dhc19vcmFjbGUucnM=) | `79.7% <100.0%> (+0.2%)` | :arrow_up: | ... and [2 files with indirect coverage changes](https://app.codecov.io/gh/chainwayxyz/citrea/pull/826/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=chainwayxyz)
eyusufatik commented 3 months ago

are we sure can remove the mutex inside blockcache

kpp commented 3 months ago

Yes

rakanalh commented 3 months ago

are we sure can remove the mutex inside blockcache

I have the same concern.

What is the justification of removing the mutex? I wanted to delay my review just to check why it was there to begin with and what potential race conditions the mutex is avoiding.

kpp commented 3 months ago

What is the justification of removing the mutex?

What is the justification of having the mutex?