Skip to content

Fix double-Arc wrapping in payments benchmark under uniffi - #1132

Merged
tnull merged 1 commit into
lightningdevkit:mainfrom
juliusjulyp:fix-arc-double-wrap-benches-payments
Oct 7, 2026
Merged

tnull merged 1 commit into
lightningdevkit:mainfrom
juliusjulyp:fix-arc-double-wrap-benches-payments

Conversation

@juliusjulyp

Copy link
Copy Markdown
Contributor

benches/payments.rs fails to compile with --all-features (which enables uniffi):

error[E0308]: arguments to this function are incorrect
   --> benches/payments.rs:177:15
    |
177 |   total += send_payments(node_a, node_b).await;
    = note: expected struct `Arc<ldk_node::Node>`
               found struct `Arc<Arc<ldk_node::Node>>`

setup_two_nodes_with_store returns TestNode, which is Arc<Node> under uniffi and plain Node otherwise. The benchmark then called Arc::new(node_a) unconditionally, which is correct for the non-uniffi case but wraps an existing Arc<Node> a second time under uniffi.

Changes

Replaced Arc::new(node_x) with node_x.into() (typed as Arc<Node>). This works for both shapes without any #[cfg]: Node -> Arc<Node> via From<T> for Arc<T>, and Arc<Node> -> Arc<Node> via the reflexive From<T> for T. This is the same approach the other tests already use for NodeEntropy (.into() ).

Testing

  • cargo check --bench payments --all-features (previously failed with the error above, now compiles)
  • cargo check --bench payments (default features, still compiles)

Signed-off-by: julypjulius <julypjulius@gmail.com>
@ldk-reviews-bot

ldk-reviews-bot commented Oct 7, 2026 •

Copy link
Copy Markdown

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 October 7, 2026 07:36

@tnull tnull left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks!

@tnull
tnull merged commit a51a120 into lightningdevkit:main Oct 7, 2026
17 of 25 checks passed
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