Move Balance out of keychain

[acerbisgianluca]

Description

Closes #1173. This PR makes Balance available at the top-level of the chain crate.

Changelog notice

All Submissions:

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

[evanlinjin]

Thank you for taking a crack at this! However, we want to move the actual definition of Balance to lib.rs. Currently, you are just doing a re-export.

[acerbisgianluca]

Hey @evanlinjin, thanks for your feedback!

I'm new to Rust development, but I see that lib.rs basically contains only modules and re-exports (correct me if I'm wrong obv), so should I move the whole Balance definiton to lib.rs anyway? I think that the issue is about re-exporting it.

[evanlinjin]

My bad, I see the issue seems to suggest reexporting. However, I don't think Balance belongs in the keychain module at all as it's not keychain-specific?

cc. @danielabrozzoni

[danielabrozzoni]

Sorry @evanlinjin, I initially thought you meant to just re-export Balance, my issue was wrongly phrased.

I agree with Evan that having Balance inside keychain.rs is weird, as there's no keychain-related method that uses the Balance! (When I created the issue I totally thought that was the case, which shows how little I know about bdk_chain still 😅)

I also agree with @acerbisgianluca that having it inside lib.rs is a bit weird, since it would be the only struct defined there.

From a quick git grep I see that the only structure using the balance is the TxGraph (as it should be), @evanlinjin should we move Balance inside src/tx_graph.rs? Otherwise, having it in lib.rs is fine as well for me.

[evanlinjin]

After a chat with @danielabrozzoni, we concluded that it'll be better to defer these decisions.

I also mentioned that I'm also not a fan of the tx_data_traits.rs and chain_data.rs files (there doesn't seem to be a clear enough distinction between the types declared in each).

Let's continue discussion on the ticket and make sure we all agree before moving forwards. Please also provide your thoughts there!

[notmandatory]

Moved to alpha.4 since this is a functional change we should figure out before the beta release.

[wthrajat]

Hi @evanlinjin!

Let's continue discussion on the ticket and make sure we all agree before moving forwards. Please also provide your thoughts there!

Have you reached to a conclusion?

[evanlinjin]

@acerbisgianluca we have reached a final conclusion for #1173

Remove the tx_data_traits.rs file and move all of it's contents into chain_data.rs. Also move the Balance struct into chain_data.rs.

Please update this PR when you have time!

[notmandatory]

A small change but I think we should push to 2.0 milestone.

[notmandatory]

Closing this one as it's replaced by #1493.