fix(starknet_gateway): convert mempool errors to the correct gw error #2995
Conversation
|
This change is |
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Graphite Automations"Yair - Auto-assign" took an action on this PR • (12/29/24)1 assignee was added to this PR based on Yair's automation. |
|
@giladchase @ShahakShama Code quote: MempoolError::P2pPropagatorClientError { .. }
| MempoolError::TransactionNotFound { .. } => {
// These errors are not expected to happen within the gateway, only from other
// mempool clients.
unreachable!("Unexpected mempool error in gateway context: {}", mempool_error);
} |
There was a problem hiding this comment.
Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @giladchase, @ShahakShama, and @yair-starkware)
crates/starknet_gateway/src/errors.rs line 81 at r1 (raw file):
} pub fn mempool_client_err_to_gw_spec_err(value: MempoolClientError) -> GatewaySpecError {
Can this be a From impl?
|
Previously, alonh5 (Alon Haramati) wrote…
No, because neither |
There was a problem hiding this comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @giladchase and @ShahakShama)
crates/starknet_gateway/src/errors.rs line 99 at r1 (raw file):
Previously, yair-starkware (Yair) wrote…
@giladchase @ShahakShama
Please approve thatP2pPropagatorClientError&TransactionNotFoundare not supposed to happen in any GW flow to your best knowledge.
TransactionNotFound can't happen in the add_tx flow AFAICT. @ShahakShama Can you confirm for P2pPropagatorClientError?
There was a problem hiding this comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @giladchase)
crates/starknet_gateway/src/errors.rs line 99 at r1 (raw file):
Previously, alonh5 (Alon Haramati) wrote…
TransactionNotFoundcan't happen in theadd_txflow AFAICT. @ShahakShama Can you confirm forP2pPropagatorClientError?
It can happen. I'll create a PR to change it so that it can't happen
There was a problem hiding this comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @giladchase)
crates/starknet_gateway/src/errors.rs line 99 at r1 (raw file):
Previously, ShahakShama wrote…
It can happen. I'll create a PR to change it so that it can't happen
After #3001 is merged it can't happen
yair-starkware
force-pushed
the
yair/fix_mempool_error
branch
from
41e11f0 to
4ec8637
Compare
There was a problem hiding this comment.
Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @alonh5 and @giladchase)
crates/starknet_gateway/src/errors.rs line 99 at r1 (raw file):
Previously, ShahakShama wrote…
After #3001 is merged it can't happen
Converted the error to a warning
|
Previously, yair-starkware (Yair) wrote…
As we talked in slack |
There was a problem hiding this comment.
Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @giladchase and @yair-starkware)
crates/starknet_gateway/src/errors.rs line 81 at r2 (raw file):
} pub fn mempool_client_result_to_gw_spec_result(
Add a short doc?
There was a problem hiding this comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @alonh5 and @giladchase)
crates/starknet_gateway/src/errors.rs line 81 at r2 (raw file):
Previously, alonh5 (Alon Haramati) wrote…
Add a short doc?
Done.
yair-starkware
force-pushed
the
yair/fix_mempool_error
branch
from
4ec8637 to
ef157e6
Compare
yair-starkware
force-pushed
the
yair/fix_mempool_error
branch
from
ef157e6 to
8711296
Compare
There was a problem hiding this comment.
Reviewed 2 of 2 files at r3, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @giladchase)
No description provided.