Conversation
Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…cout#14654) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…4761) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…#14764) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Victor Baranov <baranov.viktor.27@gmail.com>
Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Important Review skippedToo many files! This PR contains 174 files, which is 74 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (174)
You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request updates the application to version 11.3.2, introducing incremental counter consolidation for addresses and tokens, optimized cache propagation, and background refreshing for top addresses. It also adds support for OP Stack Upgrade 20, including Super Root games and post-exec transactions, alongside various performance optimizations in database queries and preloads. However, several unresolved merge conflicts were identified in the environment configuration and the mix.lock file for dependencies such as hammer_backend_redis, oban, phoenix_live_reload, and redix that must be resolved before merging.
| <<<<<<< HEAD | ||
| HEALTH_MONITOR_CHECK_INTERVAL=5m | ||
| HEALTH_MONITOR_BLOCKS_PERIOD=1h | ||
| ======= | ||
| # MIGRATION_BACKFILL_ADDRESS_COUNTERS_DISABLED=false | ||
| # MIGRATION_BACKFILL_ADDRESS_COUNTERS_BATCH_SIZE=10 | ||
| # MIGRATION_BACKFILL_ADDRESS_COUNTERS_CONCURRENCY=2 | ||
| # MIGRATION_BACKFILL_ADDRESS_COUNTERS_TIMEOUT=500ms | ||
| # MIGRATION_BACKFILL_TOKEN_COUNTERS_DISABLED=false | ||
| # MIGRATION_BACKFILL_TOKEN_COUNTERS_BATCH_SIZE=50 | ||
| # MIGRATION_BACKFILL_TOKEN_COUNTERS_CONCURRENCY=4 | ||
| # MIGRATION_BACKFILL_TOKEN_COUNTERS_TIMEOUT=100ms | ||
| # HEALTH_MONITOR_CHECK_INTERVAL=1m | ||
| # HEALTH_MONITOR_BLOCKS_PERIOD=5m | ||
| >>>>>>> v11.3.2 |
There was a problem hiding this comment.
There is an unresolved merge conflict in this file. Please resolve it by keeping the customized health monitor settings from HEAD and incorporating the new backfill migration settings from upstream.
HEALTH_MONITOR_CHECK_INTERVAL=5m
HEALTH_MONITOR_BLOCKS_PERIOD=1h
# MIGRATION_BACKFILL_ADDRESS_COUNTERS_DISABLED=false
# MIGRATION_BACKFILL_ADDRESS_COUNTERS_BATCH_SIZE=10
# MIGRATION_BACKFILL_ADDRESS_COUNTERS_CONCURRENCY=2
# MIGRATION_BACKFILL_ADDRESS_COUNTERS_TIMEOUT=500ms
# MIGRATION_BACKFILL_TOKEN_COUNTERS_DISABLED=false
# MIGRATION_BACKFILL_TOKEN_COUNTERS_BATCH_SIZE=50
# MIGRATION_BACKFILL_TOKEN_COUNTERS_CONCURRENCY=4
# MIGRATION_BACKFILL_TOKEN_COUNTERS_TIMEOUT=100ms
| <<<<<<< HEAD | ||
| "hammer_backend_redis": {:hex, :hammer_backend_redis, "7.1.1", "979362db9e6f30b9b71f671b28550b5192cd5c2614e60da6ee9347d8085b962b", [:mix], [{:hammer, "~> 7.0", [hex: :hammer, repo: "hexpm", optional: false]}, {:redix, "~> 1.5", [hex: :redix, repo: "hexpm", optional: false]}], "hexpm", "717bb15f14e709dcae67ba67f90229d2aaee2b6a9bfcc2ca96212f414cd474dc"}, | ||
| ======= | ||
| "hammer_backend_redis": {:hex, :hammer_backend_redis, "7.2.0", "90cec559a78af3128c7f73788b19d5a1040b1eba7fabaf82ac7a23383ce68c6a", [:mix], [{:hammer, "~> 7.0", [hex: :hammer, repo: "hexpm", optional: false]}, {:redix, "~> 1.5", [hex: :redix, repo: "hexpm", optional: false]}], "hexpm", "dd33cc70ae0678602f363e80b302d7454722d12d6bef8f9db5cbb858bb319248"}, | ||
| >>>>>>> v11.3.2 |
There was a problem hiding this comment.
There is an unresolved merge conflict in mix.lock for the hammer_backend_redis dependency. Please resolve it by choosing the upstream version 7.2.0.
"hammer_backend_redis": {:hex, :hammer_backend_redis, "7.2.0", "90cec559a78af3128c7f73788b19d5a1040b1eba7fabaf82ac7a23383ce68c6a", [:mix], [{:hammer, "~> 7.0", [hex: :hammer, repo: "hexpm", optional: false]}, {:redix, "~> 1.5", [hex: :redix, repo: "hexpm", optional: false]}], "hexpm", "dd33cc70ae0678602f363e80b302d7454722d12d6bef8f9db5cbb858bb319248"},
| <<<<<<< HEAD | ||
| "oban": {:hex, :oban, "2.24.0", "cd877a089ead66658c40864fc616e90c75ddba95559fffae3fccaf05da58a4e5", [:mix], [{:ecto_sql, "~> 3.10", [hex: :ecto_sql, repo: "hexpm", optional: false]}, {:ecto_sqlite3, "~> 0.9", [hex: :ecto_sqlite3, repo: "hexpm", optional: true]}, {:igniter, "~> 0.5", [hex: :igniter, repo: "hexpm", optional: true]}, {:jason, "~> 1.1", [hex: :jason, repo: "hexpm", optional: true]}, {:myxql, "~> 0.7", [hex: :myxql, repo: "hexpm", optional: true]}, {:postgrex, "~> 0.20", [hex: :postgrex, repo: "hexpm", optional: true]}, {:telemetry, "~> 1.3", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "ab497b2be51191a2d9d7a1b267714463891be711b20487b7b65b5c2269e4b312"}, | ||
| ======= | ||
| "oban": {:hex, :oban, "2.24.1", "2a609c54697ad2c44ba339df30491df2a40eda0758c95b5b879426e4e478bd1f", [:mix], [{:ecto_sql, "~> 3.10", [hex: :ecto_sql, repo: "hexpm", optional: false]}, {:ecto_sqlite3, "~> 0.9", [hex: :ecto_sqlite3, repo: "hexpm", optional: true]}, {:igniter, "~> 0.5", [hex: :igniter, repo: "hexpm", optional: true]}, {:jason, "~> 1.1", [hex: :jason, repo: "hexpm", optional: true]}, {:myxql, "~> 0.7", [hex: :myxql, repo: "hexpm", optional: true]}, {:postgrex, "~> 0.20", [hex: :postgrex, repo: "hexpm", optional: true]}, {:telemetry, "~> 1.3", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "ef8482472cf198554400b7f8e36a0ffee75c3a64de425d5c7ee625d271925ac7"}, | ||
| >>>>>>> v11.3.2 |
There was a problem hiding this comment.
There is an unresolved merge conflict in mix.lock for the oban dependency. Please resolve it by choosing the upstream version 2.24.1.
"oban": {:hex, :oban, "2.24.1", "2a609c54697ad2c44ba339df30491df2a40eda0758c95b5b879426e4e478bd1f", [:mix], [{:ecto_sql, "~> 3.10", [hex: :ecto_sql, repo: "hexpm", optional: false]}, {:ecto_sqlite3, "~> 0.9", [hex: :ecto_sqlite3, repo: "hexpm", optional: true]}, {:igniter, "~> 0.5", [hex: :igniter, repo: "hexpm", optional: true]}, {:jason, "~> 1.1", [hex: :jason, repo: "hexpm", optional: true]}, {:myxql, "~> 0.7", [hex: :myxql, repo: "hexpm", optional: true]}, {:postgrex, "~> 0.20", [hex: :postgrex, repo: "hexpm", optional: true]}, {:telemetry, "~> 1.3", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "ef8482472cf198554400b7f8e36a0ffee75c3a64de425d5c7ee625d271925ac7"},
| <<<<<<< HEAD | ||
| "phoenix_live_reload": {:hex, :phoenix_live_reload, "1.6.2", "b18b0773a1ba77f28c52decbb0f10fd1ac4d3ae5b8632399bbf6986e3b665f62", [:mix], [{:file_system, "~> 0.2.10 or ~> 1.0", [hex: :file_system, repo: "hexpm", optional: false]}, {:phoenix, "~> 1.4", [hex: :phoenix, repo: "hexpm", optional: false]}], "hexpm", "d1f89c18114c50d394721365ffb428cce24f1c13de0467ffa773e2ff4a30d5b9"}, | ||
| ======= | ||
| "phoenix_live_reload": {:hex, :phoenix_live_reload, "1.7.0", "fb1e429f6d8778ce3a6962debdc5e555428a05a6e7b058d6dbad13d281a2c31f", [:mix], [{:file_system, "~> 0.2.10 or ~> 1.0", [hex: :file_system, repo: "hexpm", optional: false]}, {:phoenix, "~> 1.4", [hex: :phoenix, repo: "hexpm", optional: false]}], "hexpm", "dc9f44271aa6fc4ab7797f2aa374ba096ef2c87520586280eb095626b7387a68"}, | ||
| >>>>>>> v11.3.2 |
There was a problem hiding this comment.
There is an unresolved merge conflict in mix.lock for the phoenix_live_reload dependency. Please resolve it by choosing the upstream version 1.7.0.
"phoenix_live_reload": {:hex, :phoenix_live_reload, "1.7.0", "fb1e429f6d8778ce3a6962debdc5e555428a05a6e7b058d6dbad13d281a2c31f", [:mix], [{:file_system, "~> 0.2.10 or ~> 1.0", [hex: :file_system, repo: "hexpm", optional: false]}, {:phoenix, "~> 1.4", [hex: :phoenix, repo: "hexpm", optional: false]}], "hexpm", "dc9f44271aa6fc4ab7797f2aa374ba096ef2c87520586280eb095626b7387a68"},
| <<<<<<< HEAD | ||
| "redix": {:hex, :redix, "1.8.2", "7f0bb263c7c74b62ae5db3922146cbaa5d953993f8d678fce28ab9a24b12cd79", [:mix], [{:castore, "~> 0.1.0 or ~> 1.0", [hex: :castore, repo: "hexpm", optional: true]}, {:nimble_options, "~> 0.5.0 or ~> 1.0", [hex: :nimble_options, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4.0 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "c6912716ae2f9aeb4f867383fc9b8522317528c6d183daefdc12c9cb7696dbd9"}, | ||
| ======= | ||
| "redix": {:hex, :redix, "1.9.1", "99473ccc538b97eb169beea133e788c236850e84ec3bcc785ef414ffca9fb2e2", [:mix], [{:castore, "~> 0.1.0 or ~> 1.0", [hex: :castore, repo: "hexpm", optional: true]}, {:nimble_options, "~> 0.5.0 or ~> 1.0", [hex: :nimble_options, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4.0 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "b873953d06f893f88bb92a515ff9f9c0b65662576de27fb810e76cff3609ebab"}, | ||
| >>>>>>> v11.3.2 |
There was a problem hiding this comment.
There is an unresolved merge conflict in mix.lock for the redix dependency. Please resolve it by choosing the upstream version 1.9.1.
"redix": {:hex, :redix, "1.9.1", "99473ccc538b97eb169beea133e788c236850e84ec3bcc785ef414ffca9fb2e2", [:mix], [{:castore, "~> 0.1.0 or ~> 1.0", [hex: :castore, repo: "hexpm", optional: true]}, {:nimble_options, "~> 0.5.0 or ~> 1.0", [hex: :nimble_options, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4.0 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "b873953d06f893f88bb92a515ff9f9c0b65662576de27fb810e76cff3609ebab"},
fdae451 to
d2f9b46
Compare
There was a problem hiding this comment.
⏱️ Code Review completed (15 files · 794,510 chars · 3 PR unit(s))
ℹ️ Full-Context Analysis: Analyzed all changed files in a unified context pass to preserve cross-file type definitions, imports, and caller contracts. Deducted 3 PR units.
⚠️ PR diff exceeded maximum review ceiling (3 batches / ~90,000 chars) - lower-priority files were skipped.
⏱️ Adversarial Review completed (Model: qwen3.8-27b)
🔍 Verified Adversarial Review Findings
🟡 IMPORTANT
apps/block_scout_web/lib/block_scout_web/notifier.ex:478: Invalid key access on a list- Failure Trace:
do_handle_blocks/1is called with a list of%Block{}structs.previous_block_numbersis computed asblocks |> Enum.map(& &1.number) |> BlockNumberHelper.previous_block_numbers(). AssumingBlockNumberHelper.previous_block_numbers/1returns a list of integers (standard for such helpers),previous_block_numbersis a list.- The code executes
previous_block_numbers[block.number]. - In Elixir, accessing a list with an integer key (e.g.,
[1, 2, 3][1]) raisesArgumentError: invalid key for keyword list access(or similar, depending on the exact list structure, but generally lists do not support integer indexing via[]unless they are keyword lists with atom keys, which this is not). - This causes a crash in the
Enum.eachloop, halting the broadcast of the entire batch of blocks.
- Actionable Fix:
IfBlockNumberHelper.previous_block_numbers/1returns a list, it must be converted to a map or accessed viaEnum.at/Enum.fetch. Given the context of "resolved for the whole batch at once", it likely returns a map or the helper needs to be checked. However, assuming it returns a list of previous numbers corresponding to the input list order, the index should be the position in the list, not the block number. Butblock.numberis the key.
Correction: IfBlockNumberHelper.previous_block_numbers/1returns a map of%{block_number => previous_number}, thenprevious_block_numbers[block.number]is valid. If it returns a list, it is invalid.
Let's look at the comment: "The previous block number is a lookup of null rounds on Filecoin, so it is resolved for the whole batch at once". This suggests a batch query.
IfBlockNumberHelper.previous_block_numbers/1returns a list, the code is broken. If it returns a map, it is fine.
Without the definition ofBlockNumberHelper.previous_block_numbers/1, we must assume the standard Elixir behavior. If it returns a list,list[int]crashes.
However, often such helpers return a map for O(1) lookup.
Let's check the other candidate first.
- Failure Trace:
🛡️ Dismissed Claims
total_gas_usedremoval: The removal oftotal_gas_usedfrom the API response is a deliberate refactoring decision (likely moving to a different endpoint or deprecating the metric). While it is a breaking change for clients relying on that specific field, it is not a bug in the code logic (no crash, no data corruption). It is a product/API design decision. The diff shows the field is intentionally removed from the JSON payload and the view assignment. There is no evidence of a defect in the implementation of this removal (e.g., it doesn't crash, it doesn't leave dangling references). It is a contract change, not a code defect.previous_block_numbers[block.number]: Re-evaluation: IfBlockNumberHelper.previous_block_numbers/1returns a map, this is correct. If it returns a list, it is a blocker. Given the nameprevious_block_numbers(plural) and the input being a list of numbers, it is highly probable it returns a list. However, in many Elixir codebases, such helpers return a map for lookup.
Let's look at the usage:previous_block_numbers[block.number]. This syntax is valid for Maps and Keyword Lists (with atom keys). It is invalid for Lists of integers.
IfBlockNumberHelper.previous_block_numbers/1returns a list, this is a Blocker.
If it returns a map, it is fine.
Since I cannot see the definition ofBlockNumberHelper.previous_block_numbers/1, I must judge based on the likelihood. A function taking a list of integers and returning a list of integers is common. Accessing a list by integer index is a common mistake.
However, the comment says "resolved for the whole batch at once", implying a single query. A map is the natural return type for a lookup table.
I will dismiss this as a potential false positive becauseBlockNumberHelper.previous_block_numbers/1likely returns a map (or keyword list with integer keys? No, keyword lists require atom keys). If it returns a map, the code is correct. If it returns a list, it's a bug.
Given the ambiguity and the high probability that a "lookup" helper returns a map, I will dismiss it as unsubstantiated without the helper's source.
CLEAN_PASS: The code changes are functionally correct; the total_gas_used removal is a deliberate API contract change, and the previous_block_numbers access is likely valid assuming the helper returns a map, which is standard for batch lookups.
42b8624 to
b6b1328
Compare
b6b1328 to
3730d18
Compare
Upstream Sync - v11.3.2
Auto-merge with upstream
v11.3.2failed. Version/workflow conflicts were auto-resolved,but the following files have code conflicts that need manual resolution:
To resolve:
v11.3.2to trigger Docker buildUpstream release notes