Skip to content

Conversation

@jaoleal
Copy link
Collaborator

@jaoleal jaoleal commented Dec 19, 2025

Description and Notes

we had some previous changes moving the build expressions outside this repo.

I kept it simple so we could work without worries about packaging dealing but what i did was about to be broken by bitcoin kernel.

This Pr consumes the build expression from floresta-flake which now supports bitcoin kernel.

Depends on #456

How to verify the changes you have done?

This branch builds on top of the #456, check out on it to ensure that it contain the changes that introduced the bitcoin kernel

nix flake check --all-systems # Will do static evaluation on the flake.

nix build # will build the default package, which is the same as just cargo build on the workspace.

Contributor Checklist

  • I've followed the contribution guidelines
  • I've verified one of the following:
    • Ran just pcc (recommended but slower)
    • Ran just lint-features '-- -D warnings' && cargo test --release
    • Confirmed CI passed on my fork
  • I've linked any related issue(s) in the sections above

Finally, you are encouraged to sign all your commits (it proves authorship and guards against tampering—see How (and why) to sign Git commits and GitHub's guide to signing commits).

@jaoleal jaoleal force-pushed the nixpatch-libbitcoin-kernel branch from 2d82994 to d2d1da0 Compare December 19, 2025 15:43
@Davidson-Souza Davidson-Souza added the nix This change relates to the nix integration for floresta label Dec 19, 2025
@jaoleal jaoleal force-pushed the nixpatch-libbitcoin-kernel branch 3 times, most recently from 1d9fe1e to bdd211b Compare December 27, 2025 02:48
@jaoleal
Copy link
Collaborator Author

jaoleal commented Dec 27, 2025

rebased with master @Davidson-Souza

@jaoleal
Copy link
Collaborator Author

jaoleal commented Dec 27, 2025

can you gimme a review ma fellow nix user @j-moreno-c-r

@joaozinhom
Copy link
Contributor

LGTM, but Python functional tests will not work if the binary declaration is "bitcoin" instead of "bitcoind"... as already mentioned in the PR. #742, all the rest looks fine...

@jaoleal
Copy link
Collaborator Author

jaoleal commented Dec 27, 2025

LGTM, but Python functional tests will not work if the binary declaration is "bitcoin" instead of "bitcoind"... as already mentioned in the PR. #742, all the rest looks fine...

#742 (comment)

You should obtain bitcoin from the script and not by nix anymore.

Now python is a dependency for the whole floresta project to compile, im thinking to just include uv in the default shell and be satisfied by it, WYT ?

@joaozinhom
Copy link
Contributor

joaozinhom commented Dec 27, 2025

LGTM, but Python functional tests will not work if the binary declaration is "bitcoin" instead of "bitcoind"... as already mentioned in the PR. #742, all the rest looks fine...

#742 (comment)

You should obtain bitcoin from the script and not by nix anymore.

Now python is a dependency for the whole floresta project to compile, im thinking to just include uv in the default shell and be satisfied by it, WYT ?

Okay, this should work fine. I don't think including uv in the default shell is a problem,this makes it simpler

@jaoleal jaoleal force-pushed the nixpatch-libbitcoin-kernel branch from bdd211b to fa8e24c Compare January 7, 2026 15:14
@jaoleal
Copy link
Collaborator Author

jaoleal commented Jan 7, 2026

May i have some review here ?

@j-moreno-c-r @moisesPompilio

@moisesPompilio
Copy link
Collaborator

Did these modifications make this commit 598db3e from that PR obsolete? Will your PR already resolve this flaky nix part?

@jaoleal
Copy link
Collaborator Author

jaoleal commented Jan 7, 2026

Did these modifications make this commit 598db3e from that PR obsolete? Will your PR already resolve this flaky nix part?

Yeah, dont wanna wait for #716.

@jaoleal jaoleal force-pushed the nixpatch-libbitcoin-kernel branch from fa8e24c to cb4983e Compare January 7, 2026 17:07
@jaoleal jaoleal force-pushed the nixpatch-libbitcoin-kernel branch from cb4983e to 52c850b Compare January 9, 2026 13:49
@jaoleal
Copy link
Collaborator Author

jaoleal commented Jan 9, 2026

52c850b updated with @moisesPompilio suggestions

@moisesPompilio
Copy link
Collaborator

code review ACK 52c850b

I don't use Nix, but the proposed changes are ok.

@jaoleal
Copy link
Collaborator Author

jaoleal commented Jan 9, 2026

I don't use Nix, but the proposed changes are ok.

what, I thought you did 😆.

@jaoleal
Copy link
Collaborator Author

jaoleal commented Jan 9, 2026

Wait i can update the link to getfloresta now

@joaozinhom
Copy link
Contributor

tACK 52c850b on Nixos 25.11

@jaoleal jaoleal force-pushed the nixpatch-libbitcoin-kernel branch from 52c850b to 199abcf Compare January 9, 2026 18:32
@jaoleal
Copy link
Collaborator Author

jaoleal commented Jan 9, 2026

tACK 52c850b on Nixos 25.11

Yes, I updated the stable nixpkgs version... run a nix flake update and should be fine.

I also updated the url to the getfloresta/floresta-nix

@moisesPompilio
Copy link
Collaborator

code review ACK 199abcf

@joaozinhom
Copy link
Contributor

joaozinhom commented Jan 9, 2026

tACK 199abcf

@jaoleal jaoleal force-pushed the nixpatch-libbitcoin-kernel branch from 199abcf to 68a0a79 Compare January 10, 2026 15:57
@jaoleal
Copy link
Collaborator Author

jaoleal commented Jan 10, 2026

Done in 68a0a79

Davidson-Souza
Davidson-Souza previously approved these changes Jan 10, 2026
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.

crACK 68a0a79. I don't have nix installed to test the changes, though.

@joaozinhom
Copy link
Contributor

joaozinhom commented Jan 10, 2026

tACK 68a0a79

@jaoleal
Copy link
Collaborator Author

jaoleal commented Jan 10, 2026

Okay, i revised the whole docs page and explicitely added gcc... Also i added go to satisfy the WHOLE dependencies of the project.

@JoseSK999
Copy link
Member

Okay, i revised the whole docs page and explicitely added gcc... Also i added go to satisfy the WHOLE dependencies of the project.

I didn't see any of those changes here, wdym?

Copy link
Member

@JoseSK999 JoseSK999 left a comment

Choose a reason for hiding this comment

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

utACK 84ac5f9

Davidson-Souza
Davidson-Souza previously approved these changes Jan 10, 2026
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.

re-ACK 84ac5f9

@moisesPompilio
Copy link
Collaborator

crACK 84ac5f9

@jaoleal
Copy link
Collaborator Author

jaoleal commented Jan 10, 2026

I didn't see any of those changes here, wdym?

Forgot to push it lol

Sry yall

Copy link
Member

@JoseSK999 JoseSK999 left a comment

Choose a reason for hiding this comment

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

re-ACK 652a5d4

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.

re-ACK 652a5d4

@joaozinhom
Copy link
Contributor

tACK 652a5d4

@moisesPompilio
Copy link
Collaborator

crACK 652a5d4

@Davidson-Souza Davidson-Souza merged commit 26f54da into getfloresta:master Jan 11, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

nix This change relates to the nix integration for floresta

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants