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

Avoid dirty libwally-core submodule after build #64

Open
Sjors opened this issue Apr 13, 2022 · 4 comments
Open

Avoid dirty libwally-core submodule after build #64

Sjors opened this issue Apr 13, 2022 · 4 comments

Comments

@Sjors
Copy link
Owner

Sjors commented Apr 13, 2022

Since #54 the build script switches from libsecp256k1-zkp to vanilla libsecp256k1 (and matching the commit used by Bitcoin Core). This uses libwally-core's --enable-standard-secp configure flag (introduced in ElementsProject/libwally-core#301).

The problem is that this switch leaves the libwally-core submodule dirty.

One possible solution would be to patch libwally-core to use libsecp256k1 from a custom directory. In that case we could include it directly as a submodule in libwally-swift and ignore libwally-core's libsecp256k1-zkp submodule.

Not sure how involved this is, cc @jgriffiths.

Perhaps there's another approach that doesn't require an upstream change. The script could do some magic with symlinks, but meh...

@jgriffiths
Copy link

@Sjors

You could maybe use a secp from elsewhere by passing -I in $CFLAGS and -L in $LDFLAGS, assuming LIBADD_SECP256K1 in configure.ac doesn't need modifying to ensure you pick up the right object files. That would require that you configure and build secp separately with compatible options following the logic that wally uses to map its own state to secp's in AC_CONFIG_SUBDIRS in configure.ac.

That seems like a huge PITA though, when you can just store the HEAD commit before you change it before building, then make clean in the secp dir and check out the old HEAD again when done.

@jgriffiths
Copy link

Worth noting that the upgrade to 0.8.5 includes bip32 derivation directly from string paths, so you could remove your path parsing code in favour of using wallys if you wished to.

@Sjors
Copy link
Owner Author

Sjors commented Apr 14, 2022

Worth noting that the upgrade to 0.8.5 includes bip32 derivation directly from string paths, so you could remove your path parsing code in favour of using wallys if you wished to.

Nice, that should help clean up some code. Are you planning to implement the opposite direction as well? ElementsProject/libwally-core#241

@jgriffiths
Copy link

Are you planning to implement the opposite direction as well?

At some point, yes - there is a backlog of wally features to implement and there hasn't been a lot of time to get to them, that situation should be improving now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants