Skip to content

fix(tests): fix/remove kmd rewards failing test#2633

Merged
shamardy merged 2 commits intodevfrom
fix-kmd-rewards
Oct 7, 2025
Merged

fix(tests): fix/remove kmd rewards failing test#2633
shamardy merged 2 commits intodevfrom
fix-kmd-rewards

Conversation

@mariocynicys
Copy link
Copy Markdown
Collaborator

since the test was already duplicated of the other one above it (test_update_kmd_rewards) but only has the from addresses manually changed to trigger setting claimed_by_me: false, the test was removed all together.

noteably, we don't have a usecase ever where we get claimed_by_me is set to false. not internally, nor via the RPC. leaving the field though just for backward combatability and future extensions (maybe?).

also, unignored two other tests.

by removing it xD
the test: `test_update_kmd_rewards_claimed_not_by_me`
Copy link
Copy Markdown
Collaborator

@shamardy shamardy left a comment

Choose a reason for hiding this comment

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

A few minor issues.

Comment on lines 2496 to 2498
/// Note that we don't have a case where this boolean is set to false.
/// It persists here though for backward compatibility and potential future use cases.
claimed_by_me: bool,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Let's just remove the field IMO. The BC is nothing like a big deal (people can simply remove it from their use-case). I am not even sure if there is a real-user depending on this field.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

agreed, we better remove it all together.

cc/ @shamardy what do u think? and we can edit method names to de-reflect that claimed_by_me field.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I am fine with removing it

and adapt usages, tests, and test description
@shamardy shamardy merged commit 0f45a51 into dev Oct 7, 2025
19 of 25 checks passed
@shamardy shamardy deleted the fix-kmd-rewards branch October 7, 2025 12:46
dimxy pushed a commit that referenced this pull request Oct 8, 2025
* dev:
  fix(TPU): correct dexfee in check balance to prevent swap failures (#2600)
  fix(tests): fix/remove kmd rewards failing test (#2633)
  chore(ci): bump CI container image to debian bullseye-slim to match dev (#2641)
  chore(release): add changelog entries for v2.5.2-beta (#2639)
  chore(release): bump mm2 version to 2.5.2-beta (#2638)
  feat(ci): add macos universal2 build (#2628)
  fix(metrics): remove memory_db size metric (#2632)
  chore(rust 1.90): make CI clippy/fmt pass
  Revert "fix(ordermatch): ignore loop-back; clear on null root; reject stale keep-alives (#2580)"
  Revert "fix(orderbook): validate roots before commit (#2605)"
dimxy pushed a commit that referenced this pull request Oct 9, 2025
* dev:
  fix(TPU): correct dexfee in check balance to prevent swap failures (#2600)
  fix(tests): fix/remove kmd rewards failing test (#2633)
  chore(ci): bump CI container image to debian bullseye-slim to match dev (#2641)
  chore(release): add changelog entries for v2.5.2-beta (#2639)
  chore(release): bump mm2 version to 2.5.2-beta (#2638)
  feat(ci): add macos universal2 build (#2628)
  fix(metrics): remove memory_db size metric (#2632)
  fix(zcoin): exact-anchor witnesses in wasm get_spendable_notes (#2629)
  fix(evm-swapv2): no mempool inclusion required for maker payment validation (#2618)
  chore(rust 1.90): make CI clippy/fmt pass
  Revert "fix(ordermatch): ignore loop-back; clear on null root; reject stale keep-alives (#2580)"
  Revert "fix(orderbook): validate roots before commit (#2605)"
dimxy pushed a commit that referenced this pull request Oct 15, 2025
`test_update_kmd_rewards_claimed_not_by_me` was removed while un-ignoring two other tests that provide coverage for kmd rewards.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants