OP_CHECKTEMPLATEVERIFY, OP_CHECKSIGFROMSTACK(VERIFY), OP

reardencode commented at 6:35 pm on January 7, 2024: none

This pull request contains a the same implementation of OP_CHECKTEMPLATEVERIFY (BIP119) as @jamesob’s Covenant Tools, an implementation of OP_CHECKSIGFROMSTACK(VERIFY) and of OP_INTERNALKEY.

There are no testnet or mainnet activation parameters proposed in this pull request. I am deeply uninterested in the details of activation semantics.

This combination of changes allows for the implementation of a variety of layer two proposals and improvements. Including, but not limited to, Lightning Symmetry, Point-Time-Locked-Contracts, Timeout Trees, and unidirectional non-interactive channels.

For anyone interested, I also have a branch with these same changes based on the latest release.

DrahtBot commented at 6:35 pm on January 7, 2024: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept NACK	michaelfolkson
Concept ACK	jonatack, moonsettler, hsjoberg, niteshbalusu11, 1440000bytes, chrisguida

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#29547 (kernel: chainparams updates for 27.x by fanquake)
#29280 (Implement OP_CHECKTEMPLATEVERIFY by reardencode)
#29270 (Implement OP_CHECKSIGFROMSTACK(VERIFY) by reardencode)
#29269 (Add OP_INTERNALKEY for Tapscript by reardencode)
#29247 (Reenable OP_CAT by 0xBEEFCAF3)
#29221 (Implement 64 bit arithmetic op codes in the Script interpreter by Christewart)
#29134 (Compressed Bitcoin Transactions by TomBriar)
#29050 (Add OP_TXHASH and OP_CHECKTXHASHVERIFY opcodes by stevenroose)
#29039 (versionbits refactoring by ajtowns)
#28806 (rpc: Add script verification flags to getdeploymentinfo by ajtowns)
#28710 (Remove the legacy wallet and BDB dependency by achow101)
#28334 (policy: Allow non-standard scripts with -acceptnonstdtxn=1 by ajtowns)
#27433 (getblocktemplate improvements for segwit and sigops by Sjors)
#26840 (refactor: importpubkey, importprivkey, importaddress, importmulti, and importdescriptors rpc by KolbyML)
#26201 (Remove Taproot activation height by Sjors)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

michaelfolkson commented at 6:47 pm on January 7, 2024: contributor

This should be opened to bitcoin-inquisition rather than this repo at this stage? I thought that was the whole point of bitcoin-inquisition.

I’m interested in why you think LN-Symmetry would be better implemented not using APO but perhaps that discussion can be had elsewhere.

reardencode commented at 7:06 pm on January 7, 2024: none

This should be opened to bitcoin-inquisition rather than this repo at this stage? I thought that was the whole point of bitcoin-inquisition.

Let’s focus on code review. There is no strict process suggesting that code first flow through inquisition.

I’m interested in why you think LN-Symmetry would be better implemented not using APO but perhaps that discussion can be had elsewhere.

Happy to discuss on delving

DrahtBot added the label CI failed on Jan 7, 2024

in src/script/interpreter.cpp:1320 in c677b3ee9f outdated

1315+                    if (sigversion == SigVersion::BASE || sigversion == SigVersion::WITNESS_V0) return set_error(serror, SCRIPT_ERR_BAD_OPCODE);
1316+                    if (flags & SCRIPT_VERIFY_DISCOURAGE_LNHANCE) {
1317+                        return set_error(serror, SCRIPT_ERR_DISCOURAGE_OP_SUCCESS);
1318+                    }
1319+
1320+                    stack.push_back(std::vector<unsigned char>{execdata.m_internal_key.begin(), execdata.m_internal_key.end()});

jonatack commented at 8:09 pm on January 7, 2024:

In commit 6d93e3a79f42791a8caccf58dc8eb2f1981bace3, the Tidy CI task https://cirrus-ci.com/task/5701906522177536?logs=ci#L3173 wants appeasing

0script/interpreter.cpp:1320:27: error: use emplace_back instead of push_back [modernize-use-emplace,-warnings-as-errors]
1 1320 |                     stack.push_back(std::vector<unsigned char>{execdata.m_internal_key.begin(), execdata.m_internal_key.end()});
2      |                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~                                                              ~
3      |                           emplace_back(

reardencode commented at 8:43 pm on January 7, 2024:

tyvm! fixed, rebased, and a couple of formatting things fixed.

jonatack commented at 8:10 pm on January 7, 2024: member

Concept ACK

moonsettler commented at 8:36 pm on January 7, 2024: none

Concept ACK

reardencode force-pushed on Jan 7, 2024

in src/script/interpreter.cpp:594 in ec96ee5da9 outdated

651@@ -591,7 +652,42 @@ bool EvalScript(std::vector<std::vector<unsigned char> >& stack, const CScript&
652                     break;
653                 }
654 
655-                case OP_NOP1: case OP_NOP4: case OP_NOP5:

callebtc commented at 8:48 pm on January 7, 2024:

case OP_NOP1: was removed (by accident?). Should it be added back to L691? Edit: sorry, misread, already done.

reardencode commented at 8:54 pm on January 7, 2024:

Git diff. Making life unnecessarily hard sometimes.

1440000bytes commented at 9:17 pm on January 7, 2024: none

Concept ACK

CHECKSIGFROMSTACK complements CHECKTEMPLATEVERIFY and helps in ticking more boxes. INTERNALKEY saves 32 bytes in some cases.

jonatack commented at 9:29 pm on January 7, 2024: member

In commit 0ac5cf9141441ea539d2f345d0d516d24a2f77bd

0$ ./src/test/test_bitcoin -t transaction_tests
1Running 7 test cases...
2test/transaction_tests.cpp:192: error: in "transaction_tests/tx_valid": mapFlagNames is missing a script verification flag

Edit: 4371ffefef2 two commits later appears to contain the missing change. Can wait until next need to push for something else.

hsjoberg commented at 9:35 pm on January 7, 2024: contributor

Concept ACK; this would make pretty much all sides happy.

reardencode force-pushed on Jan 7, 2024

DrahtBot removed the label CI failed on Jan 7, 2024

reardencode force-pushed on Jan 8, 2024

reardencode commented at 7:27 pm on January 8, 2024: none

Force pushed to fix clean commits.

niteshbalusu11 commented at 4:36 am on January 9, 2024: none

Concept ACK.

Sjors commented at 9:05 am on January 9, 2024: member

It seems too early in the process to open a pull request like this. It’s also possible to open a pull request against your own repo-fork of Bitcoin Core, or Inquisition as suggested above.

For comparison, the first Taproot pull request to this repo #17977 was made in early 2020, which was after about year of in person workshops studying the proposal. The BIP’s themselves were first proposed in 2018. The first work-in-progress taproot branch, not a pull request, is also from around 2018.

Then again, being able to use the full Bitcoin Core CI can help in getting the code in good shape, which in turn is useful in improving the proposal. You can run that locally too in Docker. That said, a few extra (draft) PR’s for soft-fork proposals is not a significant drain on (paid for by someone) CI capacity.

Typically if a pull request depends on another pull request we mark it as draft. That’s not the case here, but it does depend on two BIP proposals that were made only two days ago. We could also use some other tag to indicate that the underlying proposal is still in the concept stage.

1440000bytes commented at 10:06 am on January 9, 2024: none

A few days back Ava Chow had mentioned on twitter that there is no pull request open for CHECKTEMPLATEVERIFY:

https://x.com/achow101/status/1742427508488729044 https://x.com/achow101/status/1742555598003048753
There were some misunderstandings about unaddressed issues related to previous pull request:

https://x.com/JeremyRubin/status/1742582215354019956
Since @reardencode was already working on CHECKSIGFROMSTACK and INTERNALKEY to be used in combination with CHECKTEMPLATEVERIFY, I think it makes sense to open this pull request in bitcoin core repository for code review.
There have been several meetings and workshops for CHECKTEMPLATEVERIFY: https://utxos.org/workshops/. I am not sure if CHECKSIGFROMSTACK and INTERNALKEY need workshops or LNHANCE however they could be done with the pull request open as well.
For comparison, #29050 was opened recently and still looks in conceptual stage with unaddressed comments on BIP, incomplete implementation and still open for code review in bitcoin core repository instead of inquisition.

Then again, being able to use the full Bitcoin Core CI can help in getting the code in good shape, which in turn is useful in improving the proposal. You can run that locally too in Docker. That said, a few extra (draft) PR’s for soft-fork proposals is not a significant drain on (paid for by someone) CI capacity.

Agree

michaelfolkson commented at 10:34 am on January 9, 2024: contributor

A few days back Ava Chow had mentioned on twitter that there is no pull request open for CHECKTEMPLATEVERIFY:

There is a pull request open (in draft) that contains CTV in addition to other opcodes and sighash flags (ie proposed consensus changes).

You don’t have to be an expert in combinatorics to realize that continuing down this path results in multiple pull requests to this repo from different authors with different combinations of opcodes and sighash flags depending on the PR author’s individual preference. That was one of the major motivations for bitcoin-inquisition where multiple opcodes and sighash flags can be activated on the default signet not creating noise in this repo and allowing both review and experimentation with new opcodes and sighash flags that may or may not ever get activated on mainnet. If for some reason bitcoin-inquisition isn’t to the PR author’s tastes (I"m not aware of any problems so far) the same can be done on a custom signet with a different fork of this repo.

I totally agree with Sjors. The approach taken here of avoiding steps other soft fork proposals have gone through and just assuming shortcuts can be taken is a massive backwards step and I suspect won’t achieve anything except creating more unnecessary noise in this repo.

reardencode closed this on Jan 9, 2024

reardencode reopened this on Jan 9, 2024

reardencode commented at 2:42 pm on January 9, 2024: none

Hi Sjors, thanks for your thoughts!

It seems too early in the process to open a pull request like this. It’s also possible to open a pull request against your own repo-fork of Bitcoin Core, or Inquisition as suggested above.

I’m not aware of a well defined process for making changes to bitcoin consensus (nor do I think there should be one), so I’m not sure how we can be too early in the process.

CTV had (a year of?) workshops and review with Jeremy Rubin about 2 years ago. OP_INTERNALKEY is a binary in terms of the concept - either we’re OK with it or we’re not, no discussion, so no reason to delay the code review. CSFS does have a few points that could be discussed (and weren’t discussed years ago on the mailing list because Taproot wasn’t active at the time) and I’d love to discuss those on my bips PR. Fortunately they’d be just a few lines of code and tests to change whichever direction we go on them.

Edit to add: CTV is already in inquisition, btw, and is the bulk of this PR.

For comparison, the first Taproot pull request to this repo #17977 was made in early 2020, which was after about year of in person workshops studying the proposal. The BIP’s themselves were first proposed in 2018. The first work-in-progress taproot branch, not a pull request, is also from around 2018.

Taproot was a new witness program version, new script system, and new signing system. It is obviously not appropriate to follow the same path here as there.

Typically if a pull request depends on another pull request we mark it as draft. That’s not the case here, but it does depend on two BIP proposals that were made only two days ago. We could also use some other tag to indicate that the underlying proposal is still in the concept stage.

I disagree that the underlying proposal is at the concept stage. CTV conceptually was settled years ago, INTERNALKEY (as I said) is a simple binary decision with no room for conceptual discussion really, and CSFS does have a few minor points that can be incorporated here easily if my decisions do not match those of the broader dev community.

I’d very much appreciate further code review here.

michaelfolkson commented at 3:02 pm on January 9, 2024: contributor

From the contributing guidelines

After conceptual agreement on the change, code review can be provided.

I can only speak for myself obviously but Concept NACK from me for this repo at the current time. This pull request should be opened to bitcoin-inquisition or a fork of Core with its own custom signet consensus rules.

moonsettler commented at 3:25 pm on January 9, 2024: none

From the contributing guidelines

After conceptual agreement on the change, code review can be provided.

I can only speak for myself obviously but Concept NACK from me for this repo at the current time. This pull request should be opened to bitcoin-inquisition or a fork of Core with its own custom signet consensus rules.

You are clearly commenting in bad faith. But in the off chance you want to pretend to be consistent, here is an other PR for you to litter with your unsolicited non-technical negativity:

https://github.com/bitcoin/bitcoin/pull/28550

michaelfolkson commented at 3:35 pm on January 9, 2024: contributor

@moonsettler: That PR is in draft, was opened to bitcoin-inquisition simultaneously and doesn’t seem to have conceptual agreement at the current time either. Everyone opening their own pull request to this repo with their own favorite combination of opcodes and sighash flags wastes people’s time. They will have to come to this pull request and tell you exactly what Sjors and I have already told you. But if you insist that is what they will have to do.

reardencode commented at 3:52 pm on January 9, 2024: none

Hi @michaelfolkson, can you enumerate a concrete dispute with my proposals (on their respective PRs) or my code here? Otherwise, I’m not sure your comments are contributing to the discussion. You’ve registered your Concept NACK.

michaelfolkson commented at 3:57 pm on January 9, 2024: contributor

@reardencode: No, registering my Concept NACK is sufficient. I’ll leave it to others to rehash what Sjors and I have already advised and you can choose to ignore them too.

DrahtBot added the label CI failed on Jan 10, 2024

Sjors commented at 7:11 pm on January 12, 2024: member

I’m not aware of a well defined process for making changes to bitcoin consensus (nor do I think there should be one), so I’m not sure how we can be too early in the process.

Here’s a good place to start: https://datatracker.ietf.org/doc/html/rfc7282

(Non draft) pull requests to Bitcoin Core are not a good way to find consensus. Opening before such consensus most likely decreases of anyone being motivated to review it.

CTV had (a year of?) workshops and review with Jeremy Rubin about 2 years ago. OP_INTERNALKEY is a binary in terms of the concept - either we’re OK with it or we’re not, no discussion, so no reason to delay the code review. CSFS does have a few points that could be discussed (and weren’t discussed years ago on the mailing list because Taproot wasn’t active at the time) and I’d love to discuss those on my bips PR. Fortunately they’d be just a few lines of code and tests to change whichever direction we go on them.

Edit to add: CTV is already in inquisition, btw, and is the bulk of this PR.

Why not focus on getting OP_CTV (close to) merged first, before adding more to it?

The first PR #21702 was closed by the author. Presumably it’s been worked on more over at Inquisition?

There’s also a draft PR #28550 for vaults (superseeding #26857), which again includes OP_CTV. Are those exactly the same commits? If so, then why not focus on reviewing and improving (the OP_CTV part of) that PR?

reardencode commented at 7:39 pm on January 12, 2024: none

I’m not aware of a well defined process for making changes to bitcoin consensus (nor do I think there should be one), so I’m not sure how we can be too early in the process.

Here’s a good place to start: https://datatracker.ietf.org/doc/html/rfc7282

(Non draft) pull requests to Bitcoin Core are not a good way to find consensus. Opening before such consensus most likely decreases of anyone being motivated to review it.

As I mentioned on my post on delving it’s not clear to me how one reaches consensus on changes to bitcoin’s consensus rules. People have also said that they need to see well reviewed and ready code before we can begin reaching consensus, so I have made ready code and asked for review.

Why not focus on getting OP_CTV (close to) merged first, before adding more to it?

I had originally intended to do just that actually, but was given the feedback privately that many folks were not interested in reviewing any ideas for consensus changes that did not enable LN-Symmetry. So here I’ve made a combined PR which enables LN-Symmetry (with fewer bytes on chain than APO, it turns out).

The first PR #21702 was closed by the author. Presumably it’s been worked on more over at Inquisition?

The code hasn’t changed except for rebasing (and adding what turned out to be unnecessary caching in response to an accidentally flawed benchmark by @jamesob) since 2021. Getting CTV ready is not an available task.

There’s also a draft PR #28550 for vaults (superseeding #26857), which again includes OP_CTV. Are those exactly the same commits? If so, then why not focus on reviewing and improving (the OP_CTV part of) that PR?

See above, the CTV part hasn’t changed since 2021. That PR includes VAULT and SIGHASH_ANYPREVOUT, this PR includes CHECKSIGFROMSTACK and INTERNALKEY, and those other parts are what need attention. This set of changes is much more limited in scope than CTV+APO+VAULT - not adding deferred checks, and not modifying existing signature checking code paths.

Thanks so much for your time in responding Sjors. What path do you personally recommend for gaining consensus on changing bitcoin’s consensus rules? I’ve heard many things from many different people and this combined proposal with ready code and BIPs is my current best attempt to move toward consensus. (Happy to continue this discussion on delving or here at your discretion)

chrisguida commented at 9:10 pm on January 12, 2024: none

Concept ACK

This PR seems to be the minimal feature set to get:

LN-Symmetry,
much better PTLCs, and
all of the powerful UTXO-sharing protocols that CTV enables

Regarding LN-Symmetry, this update will FINALLY enable symmetric channel updates and eliminate the risk of losing all your money due to restoring an old backup. It also massively simplifies the channel state machine, makes on-chain resolution in the case of uncooperative channel closes much less painful, and makes watchtowers much more efficient. There are still some mempool-pinning problems with LN-Symmetry, but my understanding is that these can be fixed down the road with additional mempool work in bitcoind and more tx hashing modes added to CTV (or other, more expressive covenants like TXHASH). Regardless, enabling the LN-Symmetry update mechanism should renew developer interest in the Lightning Network and enable much more capital to be deployed therein.

CTV+CSFS can also substitute for APO to allow much better PTLCs as described in this gist, which should provide a massive boost in privacy and efficiency for Lightning payments.

Regarding CTV, the feature I’m most interested in is Non-Interactive Lightning Channels (which requires some kind of UTXO-sharing mechanism such as darkpools or Ark), which can make the onboarding experience for new bitcoiners as magical as it was for new bitcoin users a decade ago, without resorting to custodial wallets. Namely, it allows a new bitcoin user to, for instance, send some fiat to a vendor, then receive a lightning payment in a commitment vTXO that is attached to their public key. If the pool coordinator tries to cheat, they can still do an on-chain transaction to claim their UTXO. So basically this allows new bitcoin users to defer on-chain fees until they’ve done enough transactions to justify an on-chain tx, instead of having to immediately spend an on-chain fee ($10? $100??) before they’ve even gotten a chance to try bitcoin.

I’m unaware of anyone who thinks CTV is unsafe, and I’m unaware of anyone who would be against a fork that activates LN-Symmetry, as long as it doesn’t enable drivechains like APO allegedly does. (BitVM may enable drivechains anyway, so this point may be moot).

CSFS seems to be the only vector this new featureset could potentially have unforeseen consequences, but it’s been running on both Liquid and BCH for years now, so its effects should be reasonably well understood at this point.

We’ve worked hard getting Lightning to where it is today (and LN-Symmetry has been a long time coming!), and it’s still got a long way to go, but I’m hopeful that this proposal will energize devs to continue building on Lightning and make it into the future of payments it was always meant to be.

consensus: add LNHANCE deployment params

always active on regtest, signet
never active on testnet, mainnet

64406dc4c8

Add StandardTemplateHash definition 5fd5a0c709

Add SignatureChecker method for checking DefaultCheckTemplateVerifyHash. 388d30868d

Add OP_CHECKTEMPLATEVERIFY Opcode as OP_NOP4

[TESTS] modify script_tests.json to enable OP_NOP4 as OP_CHECKTEMPLATEVERIFY

ff3b9e6ed4

Change printing of NOP4 to CheckTemplateVerify (separate commit for ease of bisecting should downstream software expect to parse NOP4 this now fails) 13cc6db71e

[Anti-DoS Caching] Precompute the DefaultCheckTemplateVerifyHash cfb7677544

[Refactor] Document and pack the fields in the PrecomputedData 7174699cb0

[Anti-DoS Caching] No longer Cache CTV Hash at input index 0 3af8cc71da

Make Bare OP_CHECKTEMPLATEVERIFY basic transactions standard 1c27aac2a5

[TESTS] Add OP_CHECKTEMPLATEVERIFY functional tests b85149db14

[TESTS] add tx_valid.json tests for BIP-119 CheckTemplateVerify 9da43b99f0

[TESTS] Add tx_invalid.json examples for BIP-119 CheckTemplateVerify 46f0aed784

[TESTS] Add CTV Hash Computation Unit Test & Mutation Tester

Co-authored-by: James O'Beirne <github@au92.org>

05d96e3e68

reardencode force-pushed on Jan 14, 2024

reardencode commented at 3:01 pm on January 14, 2024: none

Force pushed to deconflict opcodes with VAULT, add deployment info output, and slightly improve tx_invalid tests.

DrahtBot removed the label CI failed on Jan 14, 2024

in src/script/interpreter.cpp:373 in 2fa22d7a52 outdated

368+        if (!success) return true;
369+        if (sig.size() != 64) return set_error(serror, SCRIPT_ERR_SCHNORR_SIG_SIZE);
370+
371+        XOnlyPubKey pubkey{pubkey_in};
372+
373+        if (!pubkey.VerifySchnorr(msg, sig)) return set_error(serror, SCRIPT_ERR_SCHNORR_SIG);

benthecarman commented at 7:06 pm on January 14, 2024:

does this mean you can do schnorr sigs with segwit v0 and p2sh? is that intended?

benthecarman commented at 7:07 pm on January 14, 2024:

msg size is also not verifiied here

reardencode commented at 7:19 pm on January 14, 2024:

Yes, intended to support both ECDSA and BIP340 sigs in segwitv0 and legacy.

No need for message size check here because BIP340 supports arbitrary length message.

in src/script/interpreter.cpp:376 in 2fa22d7a52 outdated

371+        XOnlyPubKey pubkey{pubkey_in};
372+
373+        if (!pubkey.VerifySchnorr(msg, sig)) return set_error(serror, SCRIPT_ERR_SCHNORR_SIG);
374+    } else if (pubkey_in.size() == 33 && (pubkey_in[0] == 0x02 || pubkey_in[0] == 0x03) && (sigversion == SigVersion::BASE || sigversion == SigVersion::WITNESS_V0)) {
375+        // Pubkeys of length 33 are only constrained in legacy and segwitv0. In these script types only those beginning
376+        // with 0x02 or 0x03 are constrained. Others are unknown pubkey types.

benthecarman commented at 7:06 pm on January 14, 2024:

if schnorr is allowed in non-taproot then why forbid ecdsa for taproot?

reardencode commented at 7:21 pm on January 14, 2024:

Decision was somewhat arbitrary on my part. ECDSA doesn’t really add anything in Tapscrpit where Schnorr adds ability to use well specified MPC protocols like MuSig2 and FROST in legacy, so I settled on adding Schnorr for legacy but not adding ECDSA for Tapscript.

In Tapscript, I wonder if we’ll eventually want to fork in 33-byte 02/03 keys as x-y Schnorr, so I was a bit hesitant to consume those prefixes now.

in src/script/interpreter.cpp:387 in 2fa22d7a52 outdated

382+        if (!CheckSignatureEncoding(vchSig, flags | SCRIPT_VERIFY_LOW_S, serror)) {
383+            // serror is set
384+            return false;
385+        }
386+
387+        if (msg.size() != 32) return set_error(serror, SCRIPT_ERR_INVALID_DATA_LENGTH);

benthecarman commented at 7:07 pm on January 14, 2024:

should different message sizes be left for future upgrades

reardencode commented at 7:22 pm on January 14, 2024:

ECDSA requires 32-byte message. OP_SHA256 ahead would enable scripts to handle other data sizes. Making that explicit seems like the better option for ECDSA to enable shorter scripts when pre-hashing is not needed, but also allow pre-hashed data when needed.

reardencode commented at 7:22 pm on January 14, 2024: none

Thanks for the comments @benthecarman !

Add OP_INTERNALKEY for Tapscript

Testing is minimal, but so is the code.

2edbf80c18

Implement OP_CHECKSIGFROMSTACK(VERIFY)

Some code and ideas from Elements by stevenroose, and sanket1729
Porting help from moonsettler

Tests added to the transaction tests framework.

e7670abea9

reardencode force-pushed on Jan 14, 2024

1440000bytes commented at 1:29 pm on January 15, 2024: none

I think consensus label should be added for this pull request

reardencode marked this as a draft on Jan 16, 2024

reardencode commented at 6:34 am on January 16, 2024: none

Converted this to a draft while I work through AJ’s review on my similar PR to inquisition. Will port those changes over here and undraft when the time comes.

bitcoin deleted a comment on Jan 16, 2024

DrahtBot added the label CI failed on Jan 31, 2024

DrahtBot commented at 3:59 am on January 31, 2024: contributor

🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the documentation.

Possibly this is due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

Leave a comment here, if you need help tracking down a confusing failure.

Debug: https://github.com/bitcoin/bitcoin/runs/20473421681

DrahtBot commented at 5:07 pm on March 5, 2024: contributor

🐙 This pull request conflicts with the target branch and needs rebase.

DrahtBot added the label Needs rebase on Mar 5, 2024

DrahtBot commented at 1:18 pm on May 29, 2024: contributor

There hasn’t been much activity lately and the patch still needs rebase. What is the status here?

Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
Is it no longer relevant? ➡️ Please close.
Did the author lose interest or time to work on this? ➡️ Please close it and mark it ‘Up for grabs’ with the label, so that it can be picked up in the future.

moonsettler commented at 9:10 am on May 31, 2024: none

Is it still relevant?

yes!

Did the author lose interest or time to work on this?

opening this PR against master may not really make sense all things considered. i don’t suppose anyone is under the delusion that it will be merged.

here is a branch that will be maintained for an activation client: https://github.com/lnhance/bitcoin/tree/lnhance-26.x

DrahtBot commented at 1:38 am on August 28, 2024: contributor

⌛ There hasn’t been much activity lately and the patch still needs rebase. What is the status here?

Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
Is it no longer relevant? ➡️ Please close.
Did the author lose interest or time to work on this? ➡️ Please close it and mark it ‘Up for grabs’ with the label, so that it can be picked up in the future.

maflcko commented at 6:02 am on August 28, 2024: member

opening this PR against master may not really make sense all things considered. i don’t suppose anyone is under the delusion that it will be merged.

Ok, closing for now. Please leave a comment below if you want it re-opened.

maflcko closed this on Aug 28, 2024

OP_CHECKTEMPLATEVERIFY, OP_CHECKSIGFROMSTACK(VERIFY), OP_INTERNALKEY validation (LNHANCE) #29198

Code Coverage

Reviews

Conflicts