test: consolidate test transaction creation

[xdustinface]

• Add Transaction::empty() for creating transactions with no inputs/outputs, replacing the private create_test_transaction() in transaction_record.rs
• Remove create_transaction_to_address from key-wallet/src/test_utils/wallet.rs and replace all call sites in wallet_checker tests with Transaction::dummy
• Remove create_test_transaction from router test helpers and replace all call sites across all router test files with Transaction::dummy
• Add test_addr() helper in router test helpers to reduce verbosity

Summary by CodeRabbit

• Tests
  • Consolidated and simplified transaction test setup across suites for improved maintainability.
  • Switched tests to a shared address-based dummy-transaction approach and added an empty-transaction constructor for tests.
  • Removed multiple bespoke local test helpers in favor of unified helpers, reducing boilerplate and standardizing test data and imports.

[coderabbitai]

📝 Walkthrough

Walkthrough

Tests were refactored to replace bespoke local transaction builders with a shared dummy/address-based approach: tests now use test_addr() and Transaction::dummy(...); create_test_transaction/create_basic_transaction helpers were removed; Transaction::dummy_empty() was added to dash test utils.

Changes

Cohort / File(s)	Summary
Transaction router tests key-wallet/src/transaction_checking/transaction_router/tests/coinbase.rs, key-wallet/src/transaction_checking/transaction_router/tests/classification.rs, key-wallet/src/transaction_checking/transaction_router/tests/coinjoin.rs, key-wallet/src/transaction_checking/transaction_router/tests/asset_unlock.rs, key-wallet/src/transaction_checking/transaction_router/tests/identity_transactions.rs, key-wallet/src/transaction_checking/transaction_router/tests/routing.rs, key-wallet/src/transaction_checking/transaction_router/tests/provider.rs, key-wallet/src/transaction_checking/transaction_router/tests/standard_transactions.rs	Removed local create_basic_transaction/create_test_transaction helpers. Tests now obtain addresses via test_addr() and build transactions with dashcore::blockdata::transaction::Transaction::dummy(&addr, 0..N, &[…]). Imports simplified to use Transaction and related types.
Helpers (tests) key-wallet/src/transaction_checking/transaction_router/tests/helpers.rs	Deleted create_test_transaction; added pub fn test_addr() -> Address (uses Address::dummy(Network::Regtest, 0)). Helper functions updated to construct transactions via Transaction::dummy and use addr.script_pubkey() for credit outputs.
Managed account tests key-wallet/src/managed_account/transaction_record.rs	Removed local synthetic transaction helper and switched test usage to the new Transaction::dummy_empty() helper from dash test utils.
Wallet checker tests key-wallet/src/transaction_checking/wallet_checker.rs	Replaced create_transaction_to_address test helper with Transaction::dummy(&address, 0..1, &[amount]) in tests.
Dash test utils dash/src/test_utils/transaction.rs	Added pub fn dummy_empty() -> Transaction returning a minimal Transaction with empty inputs/outputs (version 1, lock_time 0, no payload).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 I nibble code with careful paws,

Tests now hop on tidy laws.
One test_addr and dummy seed,
Helpers trimmed to make room freed.
Hop—CI green, I munch a reed.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'test: consolidate test transaction creation' clearly and directly summarizes the main change: consolidating scattered test transaction creation helpers into unified approaches using Transaction::dummy and test_addr.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch test/create-transaction

📝 Coding Plan

• Generate coding plan for human review comments

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Tip

CodeRabbit can enforce grammar and style rules using `languagetool`.

Configure the reviews.tools.languagetool setting to enable/disable rules and categories. Refer to the LanguageTool Community to learn more.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.15%. Comparing base (b05bf70) to head (2d99c00).
⚠️ Report is 1 commits behind head on v0.42-dev.

Additional details and impacted files

@@ Coverage Diff @@## v0.42-dev #538 +/- ##
=============================================
- Coverage 66.15% 66.15% -0.01% 
=============================================
Files 311 311 Lines 64710 64681 -29 =============================================
- Hits 42812 42792 -20 + Misses 21898 21889 -9 

Flag	Coverage Δ
core	75.13% <ø> (ø)
ffi	37.14% <ø> (ø)
rpc	28.28% <ø> (ø)
spv	81.01% <ø> (+0.08%)	⬆️
wallet	65.49% <100.00%> (-0.07%)	⬇️

Files with missing lines	Coverage Δ
...y-wallet/src/managed_account/transaction_record.rs	100.00% <100.00%> (ø)
...-wallet/src/transaction_checking/wallet_checker.rs	93.15% <100.00%> (-0.30%)	⬇️

... and 6 files with indirect coverage changes

[coderabbitai]

🧹 Nitpick comments (2)

key-wallet/src/transaction_checking/transaction_router/tests/helpers.rs (1)

9-10: Parameterize the test network instead of hardcoding Regtest.

test_addr() is shared across router tests; making the network explicit avoids accidental cross-network coupling as tests evolve.

♻️ Proposed refactor

-pub fn test_addr() -> Address {- Address::dummy(Network::Regtest, 0)+pub fn test_addr(network: Network) -> Address {+ Address::dummy(network, 0)
}

As per coding guidelines, **/*.rs: Never hardcode network parameters, addresses, or keys in Rust code.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.
In `@key-wallet/src/transaction_checking/transaction_router/tests/helpers.rs`
around lines 9 - 10, The helper test_addr currently hardcodes Network::Regtest
via Address::dummy(Network::Regtest, 0); change test_addr to accept a Network
parameter (e.g., fn test_addr(network: Network) -> Address) or provide an
overload that takes Network and use Address::dummy(network, 0) so callers pass
the intended network; update all test callers of test_addr to supply the
appropriate Network to avoid implicit Regtest coupling.

key-wallet/src/transaction_checking/transaction_router/tests/provider.rs (1)

176-226: Extract a shared ProRegTx builder to remove repeated literals.

These blocks are structurally near-identical; a small helper would reduce maintenance drift and keep each test focused on the one varying field.

Also applies to: 312-362, 449-499, 648-703

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.
In `@key-wallet/src/transaction_checking/transaction_router/tests/provider.rs`
around lines 176 - 226, Extract a reusable helper that constructs the
Transaction with a ProviderRegistrationPayload to eliminate repeated literals
(e.g., create a function like build_provider_registration_tx or
pro_reg_tx_builder that returns dashcore::Transaction or
ProviderRegistrationPayload), moving repeated fields such as version,
masternode_type, collateral_outpoint, default service_address,
owner_key_hash/voting_key_hash derivation, operator_public_key conversion,
operator_reward, script_payout, inputs_hash, and default signature into that
helper; update tests at the referenced blocks to call this helper and only
override the varying field(s) (for example service_address, operator_public_key,
signature) when needed so Tx construction code in tests (the Transaction and
TransactionPayload::ProviderRegistrationPayloadType usage) is centralized and
consistent.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@key-wallet/src/transaction_checking/transaction_router/tests/helpers.rs`:
- Around line 9-10: The helper test_addr currently hardcodes Network::Regtest
via Address::dummy(Network::Regtest, 0); change test_addr to accept a Network
parameter (e.g., fn test_addr(network: Network) -> Address) or provide an
overload that takes Network and use Address::dummy(network, 0) so callers pass
the intended network; update all test callers of test_addr to supply the
appropriate Network to avoid implicit Regtest coupling.
In `@key-wallet/src/transaction_checking/transaction_router/tests/provider.rs`:
- Around line 176-226: Extract a reusable helper that constructs the Transaction
with a ProviderRegistrationPayload to eliminate repeated literals (e.g., create
a function like build_provider_registration_tx or pro_reg_tx_builder that
returns dashcore::Transaction or ProviderRegistrationPayload), moving repeated
fields such as version, masternode_type, collateral_outpoint, default
service_address, owner_key_hash/voting_key_hash derivation, operator_public_key
conversion, operator_reward, script_payout, inputs_hash, and default signature
into that helper; update tests at the referenced blocks to call this helper and
only override the varying field(s) (for example service_address,
operator_public_key, signature) when needed so Tx construction code in tests
(the Transaction and TransactionPayload::ProviderRegistrationPayloadType usage)
is centralized and consistent.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 03af26d0-d470-4956-a3a1-cf041eb2bacf

📥 Commits

Reviewing files that changed from the base of the PR and between 0c394c7 and 0077e89.

📒 Files selected for processing (12)

• dash/src/test_utils/transaction.rs
• key-wallet/src/managed_account/transaction_record.rs
• key-wallet/src/transaction_checking/transaction_router/tests/asset_unlock.rs
• key-wallet/src/transaction_checking/transaction_router/tests/classification.rs
• key-wallet/src/transaction_checking/transaction_router/tests/coinbase.rs
• key-wallet/src/transaction_checking/transaction_router/tests/coinjoin.rs
• key-wallet/src/transaction_checking/transaction_router/tests/helpers.rs
• key-wallet/src/transaction_checking/transaction_router/tests/identity_transactions.rs
• key-wallet/src/transaction_checking/transaction_router/tests/provider.rs
• key-wallet/src/transaction_checking/transaction_router/tests/routing.rs
• key-wallet/src/transaction_checking/transaction_router/tests/standard_transactions.rs
• key-wallet/src/transaction_checking/wallet_checker.rs

[ZocoLini]

something like dummy_empty or empty_dummy (i dont remember right now what pattern are we following) would be better to make it clear that this is only meant for tests

[github-actions]

This PR has merge conflicts with the base branch. Please rebase or merge the base branch into your branch to resolve them.

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@key-wallet/src/transaction_checking/transaction_router/tests/helpers.rs`:
- Around line 9-10: The helper test_addr currently hardcodes Network::Regtest;
change its signature to accept a Network parameter (e.g., pub fn
test_addr(network: Network) -> Address) and call Address::dummy(network, 0)
instead of Address::dummy(Network::Regtest, 0), then update callers in tests to
pass the desired Network; this removes the hardcoded Network::Regtest and lets
tests choose the network via the test_addr(Network::...) call.
- Line 16: The test silently truncates `inputs` by using `inputs as u8` when
constructing the dummy transaction with `Transaction::dummy(&addr, 0..inputs as
u8, &[output_value])`; change this to a checked conversion (e.g. use
`u8::try_from(inputs)` or `inputs.try_into()`) and either panic with a clear
message via `.expect("inputs must fit in u8")` or handle values >255 (e.g. split
into multiple transactions) so that oversized `inputs` do not silently truncate
and produce the wrong transaction shape; update the call site to pass the
safely-converted `u8` into `Transaction::dummy`.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: e9d1a0fb-1b75-456e-96c5-d33fd5945c3b

📥 Commits

Reviewing files that changed from the base of the PR and between 25eb46a and 2d99c00.

📒 Files selected for processing (12)

• dash/src/test_utils/transaction.rs
• key-wallet/src/managed_account/transaction_record.rs
• key-wallet/src/transaction_checking/transaction_router/tests/asset_unlock.rs
• key-wallet/src/transaction_checking/transaction_router/tests/classification.rs
• key-wallet/src/transaction_checking/transaction_router/tests/coinbase.rs
• key-wallet/src/transaction_checking/transaction_router/tests/coinjoin.rs
• key-wallet/src/transaction_checking/transaction_router/tests/helpers.rs
• key-wallet/src/transaction_checking/transaction_router/tests/identity_transactions.rs
• key-wallet/src/transaction_checking/transaction_router/tests/provider.rs
• key-wallet/src/transaction_checking/transaction_router/tests/routing.rs
• key-wallet/src/transaction_checking/transaction_router/tests/standard_transactions.rs
• key-wallet/src/transaction_checking/wallet_checker.rs

🚧 Files skipped from review as they are similar to previous changes (5)

• key-wallet/src/transaction_checking/transaction_router/tests/provider.rs
• key-wallet/src/transaction_checking/wallet_checker.rs
• key-wallet/src/transaction_checking/transaction_router/tests/coinjoin.rs
• dash/src/test_utils/transaction.rs
• key-wallet/src/managed_account/transaction_record.rs