Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rewrite bails-wallet in python #4

Open
BenWestgate opened this issue Jul 13, 2023 · 10 comments
Open

Rewrite bails-wallet in python #4

BenWestgate opened this issue Jul 13, 2023 · 10 comments
Assignees
Labels
documentation Improvements or additions to documentation enhancement New feature or request help wanted Extra attention is needed performance priority: high issues raised or encountered by 2 or more testers

Comments

@BenWestgate
Copy link
Owner

bails-wallet is longer than expected. The user interface is excellent and bug-free, however the code is not as easy to understand as it could be. The functions especially are not logically grouped or ordered making it more difficult to follow along.
As it is the most security critical file in the project it should have great readability.

It may be helpful to:

  1. split it into multiple files based on logical components or functionality.
  2. group and sort the functions logically
  3. move unused functions for non-mvp features to a new file
@BenWestgate
Copy link
Owner Author

I started a refactor and organized the functions by their earliest usage in the script.

Functions used only once were removed. Unused variables removed.

There's still a 100 line function that may be better as a library called by bails-wallet as it exclusively processes text, calls no function and returns 0 or 1.

A consistent style to function declarations would benefit.

Postpone the full refactor until L1 is feature complete as that will determine whether more functions used only once can be merged or not.

@BenWestgate BenWestgate added the help wanted Extra attention is needed label Jul 15, 2023
@BenWestgate
Copy link
Owner Author

As mentioned in #5
bails-wallet should be able to run stand-alone by any linux system with bitcoin core.

With start-up parameters or environment variables telling it where to look for blockchain data and other resources.

Fork it to a new repository if necessary.

into it's own repo so that any Bitcoin Core user on Linux can create and restore codex32 backups.

@BenWestgate
Copy link
Owner Author

This unfortunately depends on two pull requests I'm involved with from Blockstream Research.

A considerable (up to all of it) could be converted to python. But I will choose the max readability and maintainability gains per time investment.

@BenWestgate BenWestgate self-assigned this Sep 13, 2023
@BenWestgate BenWestgate added documentation Improvements or additions to documentation enhancement New feature or request priority: high issues raised or encountered by 2 or more testers performance labels Sep 13, 2023
@BenWestgate
Copy link
Owner Author

It has been decreed that bails-wallet will become a stand-alone cross platform python package. So the refactor is occurring as it becomes a python package.

It may even be possible to maintain 3 repositories bitcoin-core-on-tails just installs bitcoin core and shortcuts for it, nothing else.

bails runs bitcoin-core-on-tails and installs and runs bails-wallet plus installs bails menu

bails-wallet runs the wallet code on any platform with python and bitcoin core.

BenWestgate pushed a commit that referenced this issue Mar 24, 2024
@BenWestgate
Copy link
Owner Author

The rewrite will be complete and the bash bails-wallet removed when the L1 features have been tested and working in https://github.com/BenWestgate/Bails-Wallet

BenWestgate pushed a commit that referenced this issue May 16, 2024
Copy link

Stale issue message

@BenWestgate
Copy link
Owner Author

BenWestgate commented May 27, 2024

It may even be possible to maintain 3 repositories bitcoin-core-on-tails just installs bitcoin core and shortcuts for it, nothing else.

bails runs bitcoin-core-on-tails and installs and runs bails-wallet plus installs bails menu

bails-wallet runs the wallet code on any platform with python and bitcoin core.

@epiccurious do you support moving install-core and some of its supporting files to my "bitcoin-core-on-tails" repo? Then b could clone bitcoin-core-on-tails as a dependency.

It would have a readme with install instructions like Bails but the final result would just be Bitcoin core installed on Tails. Without any extra features beyond what you'd get installing Bitcoin core on Ubuntu from the snap store.

I think that could help close #64 "Spooky Integration"

@BenWestgate
Copy link
Owner Author

The same sort of minimum setup could be a launch parameter or a different installation script but I'm unsure the technical level of the issue opener would be satisfied by that and it would require filtering the files to copy or splitting them into 2 directories in this repo i.e. bails and bitcoin-core-on-tails

@BenWestgate BenWestgate changed the title bails-wallet could use a refactor Rewrite bails-wallet in python May 27, 2024
Copy link

Stale issue message

@BenWestgate
Copy link
Owner Author

Actively being worked on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request help wanted Extra attention is needed performance priority: high issues raised or encountered by 2 or more testers
Projects
None yet
Development

No branches or pull requests

1 participant