Skip to content

chore: move Balance out of keychain module#1272

Closed
startup-dreamer wants to merge 1 commit intobitcoindevkit:masterfrom
startup-dreamer:move_balance_out_of_keychain
Closed

chore: move Balance out of keychain module#1272
startup-dreamer wants to merge 1 commit intobitcoindevkit:masterfrom
startup-dreamer:move_balance_out_of_keychain

Conversation

@startup-dreamer
Copy link
Copy Markdown

Description

Tries to Fix: bitcoindevkit/bdk_wallet#128

Moved Balance from keychain.rs to tx_graph.rs and did the rexport for bdk::wallet module.

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing

@startup-dreamer startup-dreamer force-pushed the move_balance_out_of_keychain branch from 742d617 to 1d5a3cd Compare January 14, 2024 01:50
@evanlinjin
Copy link
Copy Markdown
Member

evanlinjin commented Jan 15, 2024

Thank you for this contribution.

However, the discussion about where to put everything in bdk_chain is not final. I rather we move everything in one PR (after we come to consensus, which has not happened yet).

For the meantime, I will provide my thoughts on the matter. I think we should have a file chain/types.rs that contains all smaller public traits and structs (replacing chain_data.rs and tx_data_traits.rs files). Balance should be in types.rs.

Also note that this is a duplicate of: #1175

@LLFourn
Copy link
Copy Markdown
Collaborator

LLFourn commented Jan 15, 2024

It would be great to avoid a types.rs. This is totally unhelpful name because everything we declare is a type. It's fine to put things in lib.rs if there's no where else it belongs.

It would be fine to put tx_data_traits.rs into chain_data.rs. I don't recall why tx_data_traits.rs was created.

@notmandatory notmandatory added the api A breaking API change label Jan 16, 2024
@startup-dreamer
Copy link
Copy Markdown
Author

Closing this in support of #1283 from This comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api A breaking API change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Merge tx_data_traits.rs into chain_data.rs.

4 participants