Skip to content

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

Merged
merged 2 commits into from
Aug 8, 2025

Conversation

qrilka
Copy link
Contributor

@qrilka qrilka commented Aug 7, 2025

No description provided.

Copy link

cloudflare-workers-and-pages bot commented Aug 7, 2025

Deploying kolme with  Cloudflare Pages  Cloudflare Pages

Latest commit: 9e3cfff
Status: ✅  Deploy successful!
Preview URL: https://a4a13435.kolme.pages.dev
Branch Preview URL: https://more-submitter-traces.kolme.pages.dev

View logs

@qrilka qrilka requested a review from psibi August 7, 2025 14:24
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: {:?}",
Copy link
Member

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 {:?} ?

Copy link
Contributor Author

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
@qrilka
Copy link
Contributor Author

qrilka commented Aug 8, 2025

@psibi could you take a look into the failed test? It doesn't look related to the PR but it's a bit worrying

@psibi
Copy link
Member

psibi commented Aug 8, 2025

@qrilka That's likely because this branch doesn't have these test changes that were done to fix the tests: 9a5364e

Can you confirm it, by applying them if it fixes the tests for you ? If yes, I wouldn't worry about it and we can deploy this on our infra.

@qrilka
Copy link
Contributor Author

qrilka commented Aug 8, 2025

With that patch I get a different local failure:

    Starting 4 tests across 26 binaries (131 tests skipped)
        FAIL [   4.867s] integration-tests::balances test_balances
  stdout ───

    running 1 test
    2025-08-08T07:31:25.943684Z  INFO kolme::common: Initialized Logging
    test test_balances ... FAILED

    failures:

    failures:
        test_balances

    test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 4.86s
    
  stderr ───
    thread 'test_balances' panicked at packages/integration-tests/src/lib.rs:16:25:
    called `Result::unwrap()` on an `Err` value: Os { code: 2, kind: NotFound, message: "No such file or directory" }
    note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
    Task panicked: task 1 panicked with message "called `Result::unwrap()` on an `Err` value: Os { code: 2, kind: NotFound, message: \"No such file or directory\" }"
    thread 'test_balances' panicked at /home/kirill/ws/fpco/kolme/packages/kolme/src/testtasks.rs:45:17:
    Test run failed due to panic

@psibi
Copy link
Member

psibi commented Aug 8, 2025

@qrilka That's probably because you are not running the localosmosis. There is a just recipe for running it.

@qrilka
Copy link
Contributor Author

qrilka commented Aug 8, 2025

Oh, that was about missing contracts, after building those tests are green locally

@qrilka
Copy link
Contributor Author

qrilka commented Aug 8, 2025

Merging, will create a PR to bump Kolme on six sigma side

@qrilka qrilka merged commit 13ab238 into six-sigma-launch Aug 8, 2025
2 of 3 checks passed
@qrilka qrilka deleted the more-submitter-traces branch August 8, 2025 07:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants