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

Add support for taproot outputs to our "input info" class #2895

Merged
merged 8 commits into from
Jan 8, 2025

Conversation

sstone
Copy link
Member

@sstone sstone commented Aug 7, 2024

This is a preparation PR for simple taproot channels, which extends InputInfo to support spending taproot outputs (to spend v0 outputs we need a redeem script, but to spend v1 taproot output we need a script tree and an internal public key).
Changes are generic (i.e. not tied to specific details of the simple taproot channels extension proposal).

@sstone sstone force-pushed the input-info-taproot branch 2 times, most recently from cf583ea to 8e147fb Compare September 10, 2024 16:06
Copy link
Contributor

@remyers remyers left a comment

Choose a reason for hiding this comment

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

Seems like pretty safe refactors. I just had one scala related question for my own information.

@sstone sstone force-pushed the input-info-taproot branch 2 times, most recently from 4163f0e to e15cb29 Compare October 1, 2024 07:39
@sstone sstone force-pushed the input-info-taproot branch 2 times, most recently from f83f006 to bde8e07 Compare October 15, 2024 13:26
@sstone sstone force-pushed the input-info-taproot branch from bde8e07 to a35921c Compare October 21, 2024 14:37
@sstone
Copy link
Member Author

sstone commented Oct 21, 2024

This PR is now based on #2747

@codecov-commenter
Copy link

codecov-commenter commented Oct 21, 2024

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 86.30137% with 50 lines in your changes missing coverage. Please review.

Project coverage is 86.02%. Comparing base (5410146) to head (a842915).
Report is 19 commits behind head on master.

Files with missing lines Patch % Lines
...in/scala/fr/acinq/eclair/channel/fsm/Channel.scala 79.03% 26 Missing ⚠️
...la/fr/acinq/eclair/transactions/Transactions.scala 92.22% 7 Missing ⚠️
...c/main/scala/fr/acinq/eclair/channel/Helpers.scala 88.67% 6 Missing ⚠️
...la/fr/acinq/eclair/channel/fsm/ErrorHandlers.scala 40.00% 6 Missing ⚠️
...ire/internal/channel/version4/ChannelCodecs4.scala 90.47% 2 Missing ⚠️
...n/scala/fr/acinq/eclair/balance/CheckBalance.scala 0.00% 1 Missing ⚠️
...q/eclair/channel/publish/ReplaceableTxFunder.scala 66.66% 1 Missing ⚠️
...la/fr/acinq/eclair/io/OpenChannelInterceptor.scala 0.00% 1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2895      +/-   ##
==========================================
- Coverage   86.19%   86.02%   -0.17%     
==========================================
  Files         224      227       +3     
  Lines       20074    20610     +536     
  Branches      813      816       +3     
==========================================
+ Hits        17302    17729     +427     
- Misses       2772     2881     +109     
Files with missing lines Coverage Δ
...core/src/main/scala/fr/acinq/eclair/Features.scala 100.00% <100.00%> (ø)
...in/scala/fr/acinq/eclair/channel/ChannelData.scala 100.00% <100.00%> (ø)
...in/scala/fr/acinq/eclair/channel/Commitments.scala 96.66% <100.00%> (-0.17%) ⬇️
...a/fr/acinq/eclair/channel/fsm/CommonHandlers.scala 89.36% <100.00%> (+4.06%) ⬆️
...inq/eclair/channel/fund/InteractiveTxBuilder.scala 93.03% <100.00%> (ø)
...air/crypto/keymanager/LocalChannelKeyManager.scala 100.00% <100.00%> (ø)
...n/scala/fr/acinq/eclair/io/PeerReadyNotifier.scala 71.75% <100.00%> (+0.21%) ⬆️
...n/scala/fr/acinq/eclair/json/JsonSerializers.scala 96.42% <ø> (ø)
...ire/internal/channel/version0/ChannelCodecs0.scala 95.85% <100.00%> (+0.02%) ⬆️
...wire/internal/channel/version0/ChannelTypes0.scala 100.00% <100.00%> (ø)
... and 15 more

... and 27 files with indirect coverage changes

@sstone sstone force-pushed the input-info-taproot branch from a35921c to d560a8c Compare November 26, 2024 13:38
@sstone sstone force-pushed the input-info-taproot branch from d560a8c to 154235d Compare December 9, 2024 14:23
@t-bast
Copy link
Member

t-bast commented Jan 7, 2025

I think this is now a safe refactoring that we should put on master to simplify dependent PRs. Since #2747 is still blocked waiting for lnd to finish their implementation, can you rebase this PR directly on master? This way we can integrate it this week.

@sstone sstone force-pushed the input-info-taproot branch from a842915 to d100069 Compare January 7, 2025 10:55
@sstone
Copy link
Member Author

sstone commented Jan 7, 2025

rebased on master (27ba60f)

@sstone sstone marked this pull request as ready for review January 7, 2025 11:09
sstone and others added 7 commits January 8, 2025 12:25
Our InputInfo class contains a tx output and the matching redeem script, which is enough to spend segwit v0 transactions.
For taproot transactions, instead of a redeem script, we need a script tree instead, and the appropriate internal pubkey.
Co-authored-by: Pierre-Marie Padiou <pm47@users.noreply.github.com>
We now use specific subtypes for segwit inputs (which include a redeem script) and taproot inputs (which include a script tree and an internal key).
Older codecs have been modified to always return a SegwitInput.
v4 codec is modified and uses an empty redeem script as a marker to specify that a script tree is being used, which makes it compatible with the current v4 codec.
@sstone sstone force-pushed the input-info-taproot branch from d100069 to d2dfd07 Compare January 8, 2025 11:59
Current (v4) codecs only handle segwit inputs. Support for taproot inputs will be added to v5 codecs.
Copy link
Member

@t-bast t-bast left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@sstone sstone merged commit db93cbe into master Jan 8, 2025
1 check passed
@sstone sstone deleted the input-info-taproot branch January 8, 2025 16:40
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.

5 participants