Sync upstream v11.2.7 (merge conflicts) - #195
Conversation
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>
…lockscout#14674) 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. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request upgrades Blockscout to version 11.2.7, introduces a lightweight transaction preview endpoint, optimizes ENS and metadata preloading using concurrent requests, and implements a pooled HTTP client using Finch to reuse connections. It also adds caching for contract methods and fixes state changes for Eden sponsored transactions. The review feedback highlights critical issues, including unresolved merge conflicts in common-blockscout.env and mix.lock, a bug in HttpClient where Finch.Error is not properly rescued, and several potential runtime crashes due to missing safety checks for nil values or unconfigured environment variables.
| <<<<<<< HEAD | ||
| CONTRACT_ENABLE_PARTIAL_REVERIFICATION=true | ||
| ======= | ||
| # CONTRACT_ENABLE_PARTIAL_REVERIFICATION= | ||
| # CONTRACT_PROXY_EMPTY_IMPLEMENTATION_DATA_CACHE_TTL=1d | ||
| >>>>>>> v11.2.7 |
There was a problem hiding this comment.
There is an unresolved merge conflict in this environment file. Please resolve it by removing the conflict markers and keeping the desired configuration.
CONTRACT_ENABLE_PARTIAL_REVERIFICATION=true
# CONTRACT_PROXY_EMPTY_IMPLEMENTATION_DATA_CACHE_TTL=1d
| <<<<<<< HEAD | ||
| "oban": {:hex, :oban, "2.23.0", "1867d0fa4e8c7685217b02cc2632e3ee86c93da770e9029ff71304d9e62e53d7", [: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", "8e5f0cec5abecce78dd08cb14dc5438db90ec3884987b44773ce76fe60dd3f81"}, | ||
| ======= | ||
| "oban": {:hex, :oban, "2.23.1", "0b9495e28a236ca0478c80d666c58a8b2b55182731c3603c6e766e298a08342a", [: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", "a9855b9f5d87e31de3e2f46731b163f372e329613892a56c5b2aacceb50ed508"}, | ||
| >>>>>>> v11.2.7 |
There was a problem hiding this comment.
There is an unresolved merge conflict in the mix.lock file. Since this is a sync with upstream v11.2.7, please resolve it by keeping the upstream version (2.23.1) of the oban package.
"oban": {:hex, :oban, "2.23.1", "0b9495e28a236ca0478c80d666c58a8b2b55182731c3603c6e766e298a08342a", [: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", "a9855b9f5d87e31de3e2f46731b163f372e329613892a56c5b2aacceb50ed508"},
| rescue | ||
| # Finch raises when no connection frees up within the pool timeout (and for | ||
| # little else). Callers expect the hackney-era contract where a saturated | ||
| # pool is an {:error, _} to log and degrade on - a response without ENS | ||
| # names or tags - not an exception that kills the API request. | ||
| exception in RuntimeError -> {:error, exception} |
There was a problem hiding this comment.
The rescue block specifies exception in RuntimeError, but Finch raises Finch.Error (which is a custom exception, not a RuntimeError) on pool timeouts and other errors. This means pool timeouts will not be caught and will crash the API request instead of degrading gracefully as intended. You should rescue any exception using exception -> or include Finch.Error in the rescue list.
rescue
# Finch raises when no connection frees up within the pool timeout (and for
# little else). Callers expect the hackney-era contract where a saturated
# pool is an {:error, _} to log and degrade on - a response without ENS
# names or tags - not an exception that kills the API request.
exception -> {:error, exception}
| defp implementation_refetch_necessary?(%__MODULE__{} = proxy_implementations, smart_contract) do | ||
| if Enum.empty?(proxy_implementations.address_hashes) && verified_on_this_address?(smart_contract) do |
There was a problem hiding this comment.
proxy_implementations.address_hashes can be nil if the database column is NULL. Calling Enum.empty?/1 on nil will raise a Protocol.UndefinedError and crash the process. It is safer to default it to an empty list before checking.
defp implementation_refetch_necessary?(%__MODULE__{} = proxy_implementations, smart_contract) do
address_hashes = proxy_implementations.address_hashes || []
if Enum.empty?(address_hashes) && verified_on_this_address?(smart_contract) do
|
|
||
| defp implementation_refetch_necessary?(%__MODULE__{} = proxy_implementations, smart_contract) do | ||
| if Enum.empty?(proxy_implementations.address_hashes) && verified_on_this_address?(smart_contract) do | ||
| ttl = Application.get_env(:explorer, :proxy)[:empty_cached_implementation_data_ttl] || get_fresh_time_distance() |
There was a problem hiding this comment.
Accessing Application.get_env(:explorer, :proxy)[:empty_cached_implementation_data_ttl] directly can crash with a Protocol.UndefinedError if the :proxy configuration is nil or not set. It is safer to use Keyword.get/3 with a default empty list fallback.
ttl =
:explorer
|> Application.get_env(:proxy, [])
|> Keyword.get(:empty_cached_implementation_data_ttl)
|> Kernel.||(get_fresh_time_distance())
| total_size = Application.get_env(:explorer, :microservice_http_pool_size) | ||
| pool_count = Application.get_env(:explorer, :microservice_http_pool_count) |
There was a problem hiding this comment.
If :microservice_http_pool_size or :microservice_http_pool_count are not configured or are nil, Application.get_env/2 will return nil, causing an ArithmeticError in div/2. It is safer to provide default fallbacks directly in Application.get_env/3.
total_size = Application.get_env(:explorer, :microservice_http_pool_size, 1000)
pool_count = Application.get_env(:explorer, :microservice_http_pool_count, 20)
d3c3751 to
4f8c2b8
Compare
3d005dd to
e05174d
Compare
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_c58c6bc5-00c5-48d5-b950-420a8f18a9d6) |
e05174d to
07eb854
Compare
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_a9588607-baa6-4066-9ba7-06a0d0e9538b) |
07eb854 to
1425d31
Compare
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_9b37a20c-d99e-4527-a316-89a90d6aa259) |
4 participants
Upstream Sync - v11.2.7
Auto-merge with upstream
v11.2.7failed. Version/workflow conflicts were auto-resolved,but the following files have code conflicts that need manual resolution:
To resolve:
v11.2.7to trigger Docker buildUpstream release notes
Note
High Risk
Touches core API request paths, HTTP connection pooling, caching, and transaction state-change accounting. Misconfiguration or preload bugs can affect latency, correctness of balances, and microservice traffic.
Overview
Releases v11.2.7: a lightweight
GET /api/v2/transactions/{hash}/previewfor OG/social embeds (opt-in ENS, metadata, and input decode), plus a cluster of API performance and Eden correctness fixes.API latency: ENS and metadata preloads now run concurrently via
maybe_preload_ens_and_metadata. Microservice calls go through Finch keep-alive pools (MICROSERVICE_HTTP_POOL_SIZE/MICROSERVICE_HTTP_POOL_COUNT), split so long proxy requests cannot starve preloads. Transaction details load participants in one deduplicated pass (ABI omitted except where decoding needs it), address tags and existence checks are batched, and contract-method / empty-proxy results are cached longer (CONTRACT_PROXY_EMPTY_IMPLEMENTATION_DATA_CACHE_TTL).Eden: sponsored txs attribute fees to the fee payer, credit batched call recipients (including unindexed addresses), and pass
fee_payer/callsinto transaction interpretation. Indexer address/coin-balance extraction follows the same participants.Reviewed by Cursor Bugbot for commit 1425d31. Bugbot is set up for automated code reviews on this repo. Configure here.