Skip to content

MICRO: Chore/debug assert warnings #131

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 3 commits into from
May 14, 2025
Merged

Conversation

ProofOfKeags
Copy link
Contributor

@ProofOfKeags ProofOfKeags commented May 6, 2025

Description

This PR adds debug assertions to the warning logs in the CSM. This is helpful if we ever want to run the bridge in strict mode to get out ahead of potential issues, even if we think we know how to handle them.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature/Enhancement (non-breaking change which adds functionality or enhances an existing one)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactor
  • New or updated tests
  • Dependency Update

Notes to Reviewers

This is based on #128 and so it should be reviewed after. I briefly tried to rebase it directly onto main but it's not worth it.

Checklist

  • I have performed a self-review of my code.
  • I have commented my code where necessary.
  • I have updated the documentation if needed.
  • My changes do not introduce new warnings.
  • I have added (where necessary) tests that prove my changes are effective or that my feature works.
  • New and existing tests pass with my changes.

Related Issues

@ProofOfKeags ProofOfKeags force-pushed the chore/debug-assert-warnings branch 2 times, most recently from aca6067 to f98d81d Compare May 6, 2025 20:16
Copy link

codecov bot commented May 6, 2025

Codecov Report

Attention: Patch coverage is 0% with 30 lines in your changes missing coverage. Please review.

Please upload report for BASE (main@4c50f67). Learn more about missing BASE report.
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
crates/duty-tracker/src/contract_state_machine.rs 0.00% 29 Missing ⚠️
crates/tx-graph/src/pog_musig_functor.rs 0.00% 1 Missing ⚠️
@@           Coverage Diff           @@
##             main     #131   +/-   ##
=======================================
  Coverage        ?   51.75%           
=======================================
  Files           ?      147           
  Lines           ?    24714           
  Branches        ?        0           
=======================================
  Hits            ?    12791           
  Misses          ?    11923           
  Partials        ?        0           
Files with missing lines Coverage Δ
crates/tx-graph/src/peg_out_graph.rs 91.99% <ø> (ø)
crates/tx-graph/src/pog_musig_functor.rs 0.00% <0.00%> (ø)
crates/duty-tracker/src/contract_state_machine.rs 0.00% <0.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Member

@storopoli storopoli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not MICRO nor a chore PR.
It has a lot of stuff from #128.
Maybe we should wait that one to be merged then rebase?

@ProofOfKeags
Copy link
Contributor Author

This is based on #128 and so it should be reviewed after. I briefly tried to rebase it directly onto main but it's not worth it.

I know :)

@ProofOfKeags ProofOfKeags requested a review from storopoli May 7, 2025 18:36
@storopoli
Copy link
Member

This is based on #128 and so it should be reviewed after. I briefly tried to rebase it directly onto main but it's not worth it.

I know :)

My bad I've forgot to check the "note to reviewers" section.

@ProofOfKeags ProofOfKeags force-pushed the chore/debug-assert-warnings branch from f98d81d to 62e779e Compare May 7, 2025 19:13
@ProofOfKeags ProofOfKeags added P2 - Nice to Have enhancement New feature or request labels May 7, 2025
storopoli
storopoli previously approved these changes May 14, 2025
Copy link
Member

@storopoli storopoli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 62e779e

Copy link
Member

@storopoli storopoli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK c8482ec

But let's wait for @Rajil1213's review as well.

@ProofOfKeags
Copy link
Contributor Author

This doesn't need two reviews. I'm merging.

@ProofOfKeags ProofOfKeags added this pull request to the merge queue May 14, 2025
Merged via the queue into main with commit c09264a May 14, 2025
13 checks passed
@ProofOfKeags ProofOfKeags deleted the chore/debug-assert-warnings branch May 14, 2025 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request P2 - Nice to Have
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants