Skip to content

Conversation

@JoseSK999
Copy link
Contributor

What is the purpose of this pull request?

  • Bug fix
  • Documentation update
  • New feature
  • Test
  • Other: Mutable signatures

Which crates are being modified?

  • floresta-chain
  • floresta-cli
  • floresta-common
  • floresta-compact-filters
  • floresta-electrum
  • floresta-watch-only
  • floresta-wire
  • floresta
  • florestad
  • Other:

Description

The methods that conceptually mutate the state (even if using raw pointers) should take &mut self to let the compiler enforce the ownership rules correctly. This way it will be harder for us to misuse the methods. We also get rid of the #[allow(clippy::mut_from_ref)].

The only exception is BlockIndex.set_index_for_hash (because we pass a &self closure to it). This should be fine as we only use it inside add_index_entry.

Polished a bit the docstrings and fixed two conditional compilation warnings for Windows.

@Davidson-Souza Davidson-Souza added the code quality Generally improves code readability and maintainability label Jun 9, 2025
Copy link
Member

@Davidson-Souza Davidson-Souza left a comment

Choose a reason for hiding this comment

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

Concept ACK. Just a small nit and we are ready to go

The methods that conceptually mutate the state (even if using raw pointers) should take `&mut self` to let the compiler enforce the ownership rules correctly. This way it will be harder for us to misuse the methods. We also get rid of the `#[allow(clippy::mut_from_ref)]`.

The only exception is `BlockIndex.set_index_for_hash` (because we pass a `&self` closure to it). This should be fine as we only use it inside `add_index_entry`.

Polished a bit the docstrings and fixed two conditional compilation warnings for Windows.
@JoseSK999 JoseSK999 force-pushed the mutable-flat-chain-store-methods branch from 4f1c147 to d7daffe Compare June 10, 2025 14:20
Copy link
Member

@Davidson-Souza Davidson-Souza left a comment

Choose a reason for hiding this comment

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

ACK d7daffe

@Davidson-Souza Davidson-Souza merged commit c5e294a into vinteumorg:master Jun 11, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

code quality Generally improves code readability and maintainability

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants