fix: handle "transaction underpriced" and other unknown server error responses #1617
Conversation
| # Returns `:ok` so that BlockQueue can continue and do a retry, similar to a low replacement price error. | ||
| def process_submit_result(submission, {:error, %{"code" => code} = error}, _newest_mined_blknum) | ||
| when code >= -32_099 and code <= -32_000 do | ||
| _ = Logger.error("Submission #{inspect(submission)} resulted in an unknown server error: #{inspect(error)}.") |
There was a problem hiding this comment.
Do we alarm on all error log. If not let's make sure we add alarm on this, either via Datadog parsing the error message or do the direct alarm (?) from code?
There was a problem hiding this comment.
Good idea! I think we can do with an app alarm when a block submission is pending for more than 2 blocks (since we're aiming for next-block inclusion, but I'll make number configurable in case of too many false panics). Then Datadog can consume this app alarm similar to :ethereum_stalled_sync and friends.
Since we're losing this visibility because this PR is suppressing the exception, I'll add :block_submission_stalled alarm to this PR.
There was a problem hiding this comment.
Since we're losing this visibility because this PR is suppressing the exception, I'll add :block_submission_stalled alarm to this PR.
Queue observability size. Hell yeah!
| @@ -616,6 +636,11 @@ defmodule OMG.ChildChain.BlockQueue.Core do | |||
| :ok | |||
| end | |||
|
|
|||
| defp log_tx_underpriced(submission) do | |||
| _ = Logger.debug("Submission #{inspect(submission)} got rejected due to low gas price - ignored") | |||
There was a problem hiding this comment.
I again don't think debug log brings much value (probably 0 in this case as this would probably only happen on mainnet env).
unnawut
changed the title
|
Adding @InoMurko as reviewer as this relates to block submission |
| @@ -281,6 +281,17 @@ defmodule OMG.ChildChain.BlockQueue.Core do | |||
| :ok | |||
| end | |||
|
|
|||
| # https://github.com/ethereum/go-ethereum/blob/v1.9.15/core/tx_pool.go#L62-L64 | |||
| def process_submit_result( | |||
There was a problem hiding this comment.
I'm not entirely sure why do we even bother "handling" results. We don't do anything with them, we're just spending effort defining all function headers and then we ... log and return :ok?
I've never understood the purpose.
There was a problem hiding this comment.
I was wondering the same. It turns about the block queue uses these :ok, {:error, ...} returns to determine if the submission was successful or if it needs to retry the submission :(
I don't like it too but it's not a small refactor. I'm kind of unbundling this in #1575 though
|
I'll follow up with :block_submission_stalled in a separate PR Update: this tracking #1629 |
Closes #1615
Overview
Adds "transaction underpriced" and unknown server error response to block submission response handling, preventing unnecessary BlockQueue crashes.
Changes
BlockQueue.Core.process_submit_result/3code >= -32_099 and code <= -32_000server error fallbackBlockQueuealready started as part of feat: poisson regression and gas price feature flag #1575Testing
On local environments, set geth
--miner.gas_priceto a very high value e.g. 200Gwei. Then trigger a block submission. The error should be handled and logged.