Skip to content

Conversation

@chuksys
Copy link
Contributor

@chuksys chuksys commented Jan 11, 2026

Context

In 06a8108, persistence_backwards_compatibility was introduced. Currently, the from_seed_bytes constructor and the subsequent build method have diverging signatures depending on whether the uniffi feature is enabled.

Changes

This PR adjusts the integration tests to handle these signature differences:

  • Standard Build: Maintains the existing NodeEntropy stack allocation and fixed-size array input.
  • UniFFI Build: Adds logic to convert the seed to a Vec, unwrap the constructor Result, and wrap the final object in an Arc.

Why this is necessary

UniFFI requires Arc wrappers for cross-language memory management. Without this change, the integration tests fail to compile when the uniffi feature is active because the builder receives a raw struct instead of the expected shared pointer.

Adapt the NodeEntropy initialization to account for diverging
constructor signatures when the uniffi feature is active.

The UniFFI layer requires shared ownership (Arc) and dynamic byte
validation (Result/Vec) to facilitate memory management and error
handling across the FFI boundary. This change ensures the builder
receives the expected pointer type in UniFFI builds while
maintaining the zero-cost stack allocation for standard Rust usage.
@ldk-reviews-bot
Copy link

ldk-reviews-bot commented Jan 11, 2026

I've assigned @tnull as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

@ldk-reviews-bot ldk-reviews-bot requested a review from tnull January 11, 2026 13:08
Copy link
Collaborator

@tnull tnull left a comment

Choose a reason for hiding this comment

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

Ah, thanks, seems this slipped through.

One comment.

builder_new.set_storage_dir_path(storage_path);
builder_new.set_chain_source_esplora(esplora_url, None);

let node_new = builder_new.build(node_entropy).unwrap();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, rather than adding extra cfg flags above, can we just follow the pattern we did elswhere and add an into() here?

Suggested change
let node_new = builder_new.build(node_entropy.into()).unwrap();

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

Successfully merging this pull request may close these issues.

3 participants