-
Notifications
You must be signed in to change notification settings - Fork 0
Add more tracing into submitter: log blockhash and error kind when execution fails #446
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Deploying kolme with
|
Latest commit: |
9e3cfff
|
Status: | ✅ Deploy successful! |
Preview URL: | https://a4a13435.kolme.pages.dev |
Branch Preview URL: | https://more-submitter-traces.kolme.pages.dev |
let tx = | ||
signed_tx(program_id, blockhash, keypair, &data, &metas).map_err(|x| anyhow::anyhow!(x))?; | ||
|
||
match client.send_and_confirm_transaction(&tx).await { | ||
Ok(sig) => Ok(sig.to_string()), | ||
Err(e) => { | ||
tracing::error!( | ||
"Solana submitter failed to execute signed transaction: {}", | ||
e | ||
"Solana submitter failed to execute signed transaction: {}, error kind: {:?}", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should e
also be using {:?}
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem is that Display for this error uses Display of its field kind
but it uses #[error(transparent)]
hiding the actual kind of the error. Also that error includes Option<request::RpcRequest>
but I don't think we need this in traces.
Fixes CI clippy failure
@psibi could you take a look into the failed test? It doesn't look related to the PR but it's a bit worrying |
With that patch I get a different local failure:
|
@qrilka That's probably because you are not running the localosmosis. There is a just recipe for running it. |
Oh, that was about missing contracts, after building those tests are green locally |
Merging, will create a PR to bump Kolme on six sigma side |
No description provided.