-
Notifications
You must be signed in to change notification settings - Fork 37.3k
[WIP] BIP300 (Drivechains) consensus-level logic #28311
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
Conversation
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code CoverageFor detailed information about the code coverage, see the test coverage report. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. ConflictsReviewers, this pull request conflicts with the following ones:
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. |
Approach NACK; using bip300 Please don't take this personally, I love the work you do, personally I want to see a more 'trustless' method of pegouts before adding sidechain communication and spend conditions into Bitcoin Core. |
// TODO: Do we want to verify there's only one sidechain involved? BIP300 says yes, but why? | ||
|
||
// Lookup sidechain number from CTIP and ensure this is in fact a CTIP to begin with | ||
// FIXME: It might be a good idea to include the sidechain # in the tx itself somewhere? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if you had the tx commit to the sidechain number somewhere, I think you could remove it from CalculateDrivechainWithdrawInternalHash
?
@russeree
This comment seems to me like a "Concept NACK", not an "Approach NACK". Per @luke-jr's request:
In case this request was missed, or there was a misunderstanding about the request, by "just Approach ACK / constructive criticism" Luke is saying he only wants feedback on his specific approach taken to implement BIP300 in this PR. If a reviewer likes this approach, or does not like this approach and can suggest a better approach to implementing the BIP300 spec, then that is the kind of feedback Luke is looking for. On the other hand, if a reviewer has feedback about the general concept of BIP300 then Luke requests that that feedback be shared elsewhere -- may I suggest the bitcoin-dev mailing list or the new Delving Bitcoin forum? |
@luke-jr: Bitcoin has this predicate issue (since genesis) and it is necessary to perform a risk analysis about this for every proposed change. |
Approach NACK: given that drivechains has fundamental flaws unrelated to the exact implementation, there's no reason to distract Bitcoin Core development with this pull-req, even in draft form. What's holding Drivechains back is the idea itself, not the code. You haven't even bothered to write or link a high-level BIP-style overview of this take on the idea. Writing a bunch of detailed code is a waste of time. |
I refute the claim by Storkz and his fan boys that there are no risks or that all MEV risks are solved by Blind Merge Mining. Plus the peg out reintroduces having to trust, doesn't that seem scammy? |
"Despite providing and continuing this implementation, I myself do not thereby endorse or otherwise comment on the proposal itself." How do you reconcile this statement Luke. Pls don't support this misallocation of time, funds and energy. FUCK SYNTHETIC BITCOIN!!! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
handful of questions and comment
#include <algorithm> | ||
#include <cstdint> | ||
|
||
void CreateDBUndoData(CTxUndo &txundo, const uint8_t type, const COutPoint& record_id, const Coin& encoded_data) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we also need to unwind sidechain db operations (like incrementing of DBIDX_SIDECHAIN_WITHDRAW_PROPOSAL_ACKS
) in the undo data?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, I see now that CreateDbEntry
and DeleteDbEntry
create undo data, and ModifyDbEntry
calls both of them. Nevermind. Leaving this comment here for future reviewers
ModifyDBEntry(view, txundo, block_height, record_id, [change](CDataStream& s){ | ||
uint16_t counter; | ||
s >> counter; | ||
if (change < 0 && !counter) return; // may be surprising if change is <-1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I read the intent here as: the counter will never be < 0. If that's an important correctness property for the sidechain voting, would be nice to spell it out in a comment and/or reify it in an assert after the counter change on line 65
} | ||
|
||
uint256 CalculateDrivechainWithdrawInternalHash(const uint256& blinded_hash, const uint8_t sidechain_id) { | ||
// Internally, we hash the bundle_hash with the sidechain_id to avoid conflicts between sidechains |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
between this comment and the fact that the first in/out of the tx are nulled for computing the hash, it sounds like the same withdrawal tx could be provided for multiple sidechains. Is that desired?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It could be voted on because we only see the hash at that stage. But it would never be valid for more than one.
// TODO: Block is invalid if there are no bundles proposed at all | ||
// FIXME: Presumably blocks should only be able to vote once - this is missing in the BIP | ||
for (int sidechain_id = 0; sidechain_id < 0x100; ++sidechain_id) { | ||
// FIXME: bounds checking |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably something like out.scriptPubKey.size() >= 6 + (0x100 * data_format)
before we start counting votes
static constexpr uint8_t BIP300_HEADER_WITHDRAW_PROPOSE[] = {0xd4, 0x5a, 0xa9, 0x43}; // n.b. 67 byte push followed by only 32 bytes | ||
static constexpr uint8_t BIP300_HEADER_WITHDRAW_ACK[] = {0xd7, 0x7d, 0x17, 0x76}; // n.b. 23-byte push followed by variable bytes | ||
if (std::equal(&out.scriptPubKey[1], &out.scriptPubKey[5], BIP300_HEADER_WITHDRAW_ACK)) { | ||
const uint8_t data_format = out.scriptPubKey[6]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is data_format
variable? if it was something fixed, we could statically determine the block weight increase and also check the size of the scriptpubkey above (after line 94) and bail early if ifs not well-formed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree. I'm not sure what the intent is for it. It seems to be based on a mistaken assumption that serialized bytes are a resource to minimise. But for now, this implements [most of] the current BIP draft.
acks_s >> acks; | ||
} | ||
if (acks >= SIDECHAIN_WITHDRAW_THRESHOLD) { | ||
// Overwrite an existing sidechain with a new one |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we want to also explicitly delete any lingering DBIDX_SIDECHAIN_WITHDRAW_PROPOSAL_LIST
entries for the deleted sidechain?
Approach NACK You yourself have said that this will not work "without significant additional changes to handle an activation". So what's the point of writing, reviewing and merging this code without completely knowing those "significant changes" ahead of time? And surely people have opinions about Drivechain in general because the big debate around it is about mining pool centralization and valid perverse incentives(and since this will lead a way for that to happen on the Bitcoin system instead of an independent sidechain implementation those opinions are also somewhat valid given this is incomplete work). |
I have engaged Peter (and compensated him financially) to write a report on these "fundamental flaws", but I have not yet heard or seen anything. Peter, until you publish these flaws, we have no way of evaluating them. (Nor would this be the place to do that, even if you had. Per Luke's comment on Concept ACK/NACKs.) |
what exactly do you mean by trustless? what can be more trustless than hash-rate escrow? After years of research, all other approaches rely on actual centralized custodians like liquid does. Hash rate escrow leverages the assumption that sha256 hash rate is sufficiently distributed for bitcoin censorship resistance properties. |
Meta Approach ACK - i like the approach of developing an implementation, independent of the concept ack/nack having said that, regarding the concept.. We need to be laser focused (no pun intended) on scaling bitcoins utxo p2p money utility, and while the idea of a 2way peg sidechains using hash rate escrow via the solution here is to "limit" sidechains to the p2p money utility. this will keep miner incentives as is, and scale bitcoin with bitcoin script. such a solution can be achieved, but first requires this explicit social consensus intent and explicit designer and implementor intent. with such intent in place, the miners Nash equilibrium is to follow intent, due to the implied |
Approach NACK Missing analysis statement, which effects or how this proposal could increment the exploit exposure for the Bitcoin system due to the loose flexibility of Bitcoin’s script (predicative processing). Missing statement how this proposal will affect the Nash equilibrium between the distribution of affordable archival full nodes and affordable mining devices. Concerns, that equilibrium is reached by highly capitalized and centralized original node (storage + mining) entities neither addressed nor mitigated. |
Cheemsburger ACK! |
NACK. I say NJET, NADA, NIENTE, NOPE, NO, いいえ I quote @B__T__C "... the miners mine on the layer based & hold it & the users get some shit tokens or shit coins on the drive... " personally I totally agree with Pete Todd This BIP300 should not be accepted for several reasons. Another reason, the rewards system looks like a pyramid scheme if this get through. This will have technical, legal and ethical implications. This is a trojan horse code injection to fundamental things of bitcoin. And yes, belief what You want who I am. Satoshin |
Look drivechains are good for only a couple of things such as security, and
deploying a token possibly while being backed by Bitcoin. It's not a good
idea as a side chain to run simultaneously because it would slow
transactions as well as screw with the parsing and verification. How ever
creating a personal drive chain can add a layer of security but can be
messed up if you don't pay attention to how the drive chain is configured.
…On Mon, Aug 21, 2023, 6:20 PM Luke Dashjr ***@***.***> wrote:
This is a (rough draft) clean rewrite of BIP300 (Drivechain)
consensus-level code.
Instead of a separate sidechain database (which may be prone to
hard-to-review/test consistency issues), instead the unusable 8 MSB of the
UTXO index are reserved for non-UTXO database entries, and the existing
UTXO db and caching layer is shared. This can be refactored in the future,
but I think it is the cleanest and most reviewable approach initially -
open to other ideas, though. There's also some ugliness in the undo data to
handle restoring the new data, but it's abstracted and shouldn't be too
hard to reason about.
Using these new primitives, Drivechains can be reimplemented with a
UTXO-like model. Note that there is zero activation logic in the current
PR: the protocol changes are *always* active. Therefore, this will *not*
work (at least not safely) on Bitcoin today, and cannot be deployed without
significant additional changes to handle an activation.
A new SERIALIZE_TRANSACTION_FOR_WEIGHT serialization flag is also added,
that is meant only for weight counting. This allows for adjusting weight
(upward) to fit additional resource requirements by new functionality. In
this case, several Drivechain "messages" are expected to have a larger
burden than their OP_RETURN encoding would otherwise weigh. However, the
specific adjustments are not implemented in this draft.
As a consensus change, this can only be implemented with community
support. Many people seem to have opinions, but please keep them to other
forums. Despite providing and continuing this implementation, I myself do
not thereby endorse or otherwise comment on the proposal itself.
Therefore, not looking for concept ACKs/NACKs, just *Approach* ACKs /
constructive criticism.
------------------------------
You can view, comment on, or merge this pull request online at:
#28311
Commit Summary
- 7ee7f6e
<7ee7f6e>
dbwrapper: Restore `CDBIterator::GetValueSize()`
- bc6a2e1
<bc6a2e1>
txdb: Support for flags which prevent opening with incompatible software
- e0a51eb
<e0a51eb>
Hide COutPoint with n > 0x00ffffff from consensus/transaction handling
- df2c012
<df2c012>
WIP Drivechain via UTXOs
- eca69c4
<eca69c4>
primitives/transaction: Implement CTxIn::SetNull
- d4eb444
<d4eb444>
Implement OP_DRIVECHAIN-based sidechain deposits/withdraws
- 2493b15
<2493b15>
Add SERIALIZE_TRANSACTION_FOR_WEIGHT flag
- ecd75f5
<ecd75f5>
primitives/transaction: Stub weight increases for drivechains
- 4c02190
<4c02190>
sidechain: Expire sidechain/withdraw proposals
- 04783a9
<04783a9>
fixups, simplify record_id confusion in various places
- d886713
<d886713>
sidechain: Hash blinded withdraw hashes with the sidechain id to avoid
conflicts
- 7c8d7e2
<7c8d7e2>
sidechain: INCOMPLETE sidechain activation
- f544581
<f544581>
fixups, cast changes to avoid narrowing conversion warnings
- 2192ba1
<2192ba1>
sidechain: Detect several invalid conditions
- ff9815a
<ff9815a>
Add push1[sidechain id] OP_TRUE after OP_DRIVECHAIN
- ea8c1a9
<ea8c1a9>
sidechain: Setup DBIDX_SIDECHAIN_CTIP_INFO
- afc975b
<afc975b>
sidechain: Detect more edge cases and document TODOs
File Changes
(23 files <https://github.com/bitcoin/bitcoin/pull/28311/files>)
- *M* src/Makefile.am
<https://github.com/bitcoin/bitcoin/pull/28311/files#diff-4cb884d03ebb901069e4ee5de5d02538c40dd9b39919c615d8eaa9d364bbbd77>
(3)
- *M* src/coins.cpp
<https://github.com/bitcoin/bitcoin/pull/28311/files#diff-f0ed73d62dae6ca28ebd3045e5fc0d5d02eaaacadb4c2a292985a3fbd7e1c77c>
(24)
- *M* src/coins.h
<https://github.com/bitcoin/bitcoin/pull/28311/files#diff-095ce1081a930998a10b37358fae5499ac47f8cb6f25f5df5d88e920a54e0341>
(26)
- *M* src/consensus/validation.h
<https://github.com/bitcoin/bitcoin/pull/28311/files#diff-829bb21fff2a0d0648adb348a52928b2227f1cd05a54f44ce78d84091f8060d4>
(6)
- *M* src/dbwrapper.cpp
<https://github.com/bitcoin/bitcoin/pull/28311/files#diff-44bef8fa93d168379ef331d9e11d333be06ce98a8f37ec65f8a987846b90a151>
(5)
- *M* src/dbwrapper.h
<https://github.com/bitcoin/bitcoin/pull/28311/files#diff-ac5fbeb5de6f28370bc348a579fc465fe7f7b91df0e0483c6edbf10273278c0d>
(3)
- *M* src/primitives/transaction.h
<https://github.com/bitcoin/bitcoin/pull/28311/files#diff-32f83e93b123235079fbadc3f68f8158c56c67d031a579475e16c6857edbecbf>
(30)
- *M* src/rpc/blockchain.cpp
<https://github.com/bitcoin/bitcoin/pull/28311/files#diff-decae4be02fb8a47ab4557fe74a9cb853bdfa3ec0fa1b515c0a1e5de91f4ad0b>
(1)
- *M* src/script/interpreter.cpp
<https://github.com/bitcoin/bitcoin/pull/28311/files#diff-a0337ffd7259e8c7c9a7786d6dbd420c80abfa1afdb34ebae3261109d9ae3c19>
(2)
- *M* src/script/script.cpp
<https://github.com/bitcoin/bitcoin/pull/28311/files#diff-6da39bed8a76fcaae51f021f0cdd906748aa99fdbf2d41981947f6a1b18fbe7b>
(11)
- *M* src/script/script.h
<https://github.com/bitcoin/bitcoin/pull/28311/files#diff-93c5ddf703879287802d651b68c9c6199644afe23db0dd3ff77e8d1ce3c20045>
(4)
- *A* src/sidechain.cpp
<https://github.com/bitcoin/bitcoin/pull/28311/files#diff-068cca671b293f3ee6f24c0a950c8357fc42518631e8409876c810da7c03ebb9>
(466)
- *A* src/sidechain.h
<https://github.com/bitcoin/bitcoin/pull/28311/files#diff-c5edaee37a176f59597dc09ce65d926090bca357709bcfd7466f4c239ceb5d6c>
(65)
- *M* src/test/coins_tests.cpp
<https://github.com/bitcoin/bitcoin/pull/28311/files#diff-3d0856e8b7f136c588b229e0cbd3b2e2c309cd097ade0879521daba4e1bb2a33>
(29)
- *M* src/test/fuzz/coins_view.cpp
<https://github.com/bitcoin/bitcoin/pull/28311/files#diff-1ef3b6a1936b50f3d5ec4a1786d9e2d63d1a3e1815b103e67f20601995f355b4>
(14)
- *M* src/test/fuzz/coinscache_sim.cpp
<https://github.com/bitcoin/bitcoin/pull/28311/files#diff-a8f78513bc27f9bf679eead54819a8e5be720401c6ae40858da226a66ca002e2>
(10)
- *M* src/test/fuzz/tx_pool.cpp
<https://github.com/bitcoin/bitcoin/pull/28311/files#diff-07fe07ecfff4dd7c233a453006567140ff20d226fd73a20406df14982b1e415b>
(2)
- *M* src/txdb.cpp
<https://github.com/bitcoin/bitcoin/pull/28311/files#diff-cafbe1353eff6084b73fd3b6c3dee603e0827348fdd2fe12dfad1e01003a84ed>
(36)
- *M* src/txdb.h
<https://github.com/bitcoin/bitcoin/pull/28311/files#diff-d102b6032635ce90158c1e6e614f03b50e4449aa46ce23370da5387a658342fd>
(6)
- *M* src/txmempool.cpp
<https://github.com/bitcoin/bitcoin/pull/28311/files#diff-c065d4cd2398ad0dbcef393c5dfc53f465bf44723348892395fffd2fb3bac522>
(4)
- *M* src/txmempool.h
<https://github.com/bitcoin/bitcoin/pull/28311/files#diff-8a2230436880b65a205db9299ab2e4e4adb1d4069146791b5db566f3fb752ada>
(2)
- *M* src/undo.h
<https://github.com/bitcoin/bitcoin/pull/28311/files#diff-75c400fd71ce89e9dab36f9fc4d079746981836b474c2e868135c37eb5139555>
(2)
- *M* src/validation.cpp
<https://github.com/bitcoin/bitcoin/pull/28311/files#diff-97c3a52bc5fad452d82670a7fd291800bae20c7bc35bb82686c2c0a4ea7b5b98>
(51)
Patch Links:
- https://github.com/bitcoin/bitcoin/pull/28311.patch
- https://github.com/bitcoin/bitcoin/pull/28311.diff
—
Reply to this email directly, view it on GitHub
<#28311>, or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AORL367L5NQUE7TACLIEHOTXWQCODANCNFSM6AAAAAA3ZE4L3M>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
@luke-jr @petertodd As someone that can claim to be your technical peer and with experience in consensus development, I would appreciate if you can publish on the communication space of your convenience the contractual commitment excerpts linking your work on drivechain to the proposal authors themselves . The amount and all personal matters are no public business, though at the very least I hope you have minimal provisions protecting the autonomy of your creative work or ensuring the intellectual outcome respect FOSS licenses. With all my respect, I’m not questioning the legitimacy as an independent technical expert who has accumulated years of hard works of being paid to offer a grounded and in-depth technical opinion on a subject in the scope of its competence. Here I’m just asking for transparency on potential conflict of interest that one would request in all walk of life (law, health, energy) when experts are expressing themselves on a complex subject with potential large-scale and serious impact on the end-users (financial) life. Personally, I have the competence and knowledge to evaluate the proposal both in code and its associated documentation without relying on “domain experts”, though I think other folks in the community would appreciate the ethical effort. I understand it’s delicate as a remark though as Rusty Russell observed a while back in matters of open-source cryptocurrency development ethics, we’re keeping each other in check as peers. I’ve been in the same situation early last year when I’ve been approached to do a paid security review of the CTV soft-fork and I politely declined. My professional policy for years have been to decline any work commitment related to consensus changes to preserve the independence of my views. If you would like feedback on how to construct ethically and legally such agreement, I’m staying available out of band to provide my viewpoint and relevant information. I don’t think the discussion is appropriate for the GH repository. I’m staying available to discuss the ethics and ideal development process of consensus changes on the mailing list or other forum of discussions. As a community, I think we’re still paying the trauma of a group of Bitcoin dev experts raising millions in VC-funding almost a decade ago to promise new technology and corresponding consensus changes and finding themselves in a massive situation of conflict of interests with the rest of the ecosystem, whatever the great achievements they have realized on a lot of significant fronts. |
NACK, concept, approach, all. IMV Bitcoin is NOT made for the BIP/PR mania, Core Devs profit absolutely from. Any Core Devs, are by definition, not the ones to decide anything due to lack of exemption as service providers in Bitcoin, not unbiased customers/node operators(the only unbiased who must decide about their OWN property). It is for long been that Devs keep pushing node operators out of the decisions that belong solely to them as owners of Bitcoin, NOT to Devs, NOT to miners. Instead of coding every time an OPTION in full nodes software to allow (UN)SUPPORT for each non-stop new BIP/PR mania non-bug fixing CHANGE to Bitcoin, they discretely discuss things in a 'only for the experts central Hub' environment, like this one in GitHub and less user friendly, MS-DOS/command prompt like and as geeky as possible, JUST LIKE ALTCOINERS DID when inventing a lot of weird new jargon for things that always existed, so that all the idiots could eat their very shady technical jargon in speculative marvel and fiat dollar signs in their eyes. |
I emphatically do not support this Pull request. |
Approach NACK. Also the promotor does block people who debate on this issue on twitter. If You propose a BIP You defend it with arguments, not with money and propaganda. This proposal paves the way to abuse the reward system. People who contribute to mine bitcoin have the right to receive real bitcoin not some reward on a sidechain. And what happens if the sidechain gets issues with the mainchain to merge etc. This idea is totally unsafe and my eye is on stratum. |
I thought that this part was admirably clear:
This area is for discussing the Approach/Implementation. Everything else is off-topic. Any skeptics or opponents of DC should at least pretend to want to see a real implementation, before dismissing it. After all, pre-implementation the idea is inherently unfinished. |
There is an uncertainty on the open-source status of the technical contributions due to the apparent mode of financial compensation of the "champions" developers, among other issues. While this might be acceptable for a lot of contributions in other areas of the Bitcoin ecosystem, here we're talking about consensus changes in a $500B ecosystem, any exposure due to uncertain license on key components might taint the rest of the open-source Bitcoin software. I think we've already enough issues with CSW lawsuits from now on to minimize the legal exposure of all ecosystem stakeholders, in my reasonable opinion. This type of discussion happens, among other forums, on the Github repository. E.g #28175.
I've never seen any Bitcoin Core commit or substantial review on delicate critical components realized under your pseudonym, or even substantial technical contributions on deployed and with real-world usage multi-party applications or contracting protocols (e.g payment channels), so I don't know your level of understanding of the Bitcoin network, protocols architecture and ecosystem. Even when an idea is fully fleshed out in code and once it deployed you have unmeasurable security, safety and compatibility maintenance costs beared by the whole ecosystem ad vitam aeternam (i.e Personally, I don't have strong technical opinion on DC / sidechains, as with any complex technical matter this is far from being a "binary" subject. All I know is we're Aug 30 (depending your timezone) and there is far enough hard technical issues to review and test for the coming year (package relay, assumeutxo, libitcoinkernel, bip324, maybe maybe APO, transaction-relay policy improvement for data carriage usage). Speaking for myself and myself only, this is not worthy to allocate high-caliber technical review time (in a pure FOSS fashion without financial compensation to preserve independence of my judgement) on a consensus changes project with that level of uncertainty in its development process, at the very least until next year Aug 30 2024. By then, I hope project champions will have clarified the development and technical evaluation process of DC. If you would like to discuss in depth the ethics of Bitcoin consensus changes, what lessons can be sketched out from the failed activation of CTV by its champion Jeremy Rubin last year or the best development practices of critical Bitcoin components, I'm staying available to discuss it during in person at a panel or another public format at a Bitcoin conference during the coming years. |
* Change sidechain.cpp CreateDBUndoData record_id.n assertion to record_id.n > COutPoint::MAX_INDEX from <= COutPoint::MAX_INDEX
* CDataStream is invalidated by test reading the sidechain proposal so create a new stream for CreateDBEntry
* Fix CDataStream handling bug * Remove sidechain number from new_sidechains_activated set after activation
* Add functions to CScript to check for drivechain coinbase commitment script headers * Use new functions in sidechain coindb
Rebased and added some fixes |
Note that BIP-301 suffers from an equivocation attack. While there are many other reasons to NACK this pull-req, at minimum the equivocation attack is a sufficient reason alone. |
If that was an issue then we would add 1 byte... as you mention yourself "An obvious solution would be to tag the OP_Return outputs with some kind of merge-mined chain identifier, and implement a strict rule consensus rule in the blind-merge-mined chain where the first matching tag wins." It isn't an issue though. h* can and in our examples does include information about which chain it belongs to. |
As BIP-301 states, h* is a 32 byte hash digest. The problem is you can have two valid h*'s in the same coinbase, for the same chain, and without access to the data behind h*, you have no way of detecting that. |
The sidechains have access to the data behind h*. The mainchain doesn't care but it can also tell: https://github.com/LayerTwo-Labs/mainchain/blob/3f4d632a90ebd5eb5f94dba24384235043c34a69/src/validation.cpp#L3952-L3990 This is an example of one part of the BMM h* validation from the old version of our code but you get the idea. |
That code allows for additional OP_Returns with the special BMM marker to be put in the set of coinbase outputs, beyond the set of BMMs that should be present due to txs in the block itself. |
🐙 This pull request conflicts with the target branch and needs rebase. |
Peter's "equivocation attack" is just him misunderstanding BMM -- specifically wrongly assuming it is like Namecoin (as he says in his linked article). Unlike Namecoin, which is its own Blockchain and can survive on its own, BMM assumes that L1 can be found -- the L2 therefore has full access to all of L1 already. L2 already sees everything on L1, and its order. (L2, of course, already sees everything on L2. And it knows what sidechain number it is.) Unfortunately, I don't think there's anything anyone can do, to actually get Peter to spend 5 minutes engaging with Bip300/301. Instead he is determined to:
His comments at Baltic Honeybadger indicate he had never visited the top of the project site drivechain.info at all in the past 4+ years. And his tweets before TabConf indicate that he had not yet any rudimentary familiarity with how the idea worked, but was going to declare it broken anyway. Even after paying him for his time, he couldn't be bothered to actually look at the idea, he has to just fabricate something instead -- the writeup is full to the brim of these errors and fabrications. Not sure what else there is to say. |
On Wed, Nov 15, 2023 at 01:05:13PM -0800, Paul Sztorc wrote:
Peter's "equivocation attack" is just him misunderstanding BMM -- specifically wrongly assuming it is like Namecoin (as he says in his linked article).
Unlike Namecoin, which is its own Blockchain and can survive on its own, BMM assumes that L1 can be found -- the L2 therefore has full access to all of L1 already. L2 already sees everything on L1, and its order.
If that is your design assumption, forcing BMM commitments to be put in the
coinbase is redundant and accomplishes nothing. The only reason to do that is
for the sake of SPV-type validation, in which case there is a equivocation
attack.
Note that large coinbase transactions have caused a number of problems before,
including issues with ASIC compatibility and p2pool. We should not inflate
coinbase transaction size needlessly.
(L2, of course, already sees everything on L2. And it knows what sidechain number it is.)
Unfortunately, I don't think there's anything anyone can do, to actually get Peter to spend 5 minutes engaging with Bip300/301. Instead he is determined to:
1) misunderstand the idea
2) make something else up, that is inaccurate
3) criticize that
4) pat himself on the back and call it a day
His comments at Baltic Honeybadger indicate he had never visited the top of the project site drivechain.info at all in the past 4+ years. And his tweets before TabConf indicate that he had not yet any rudimentary familiarity with how the idea worked, but was going to declare it broken anyway. Even after paying him for his time, he couldn't be bothered to actually look at the idea, he has to just fabricate something instead -- the writeup is full to the brim of these errors and fabrications.
Given that I'm far from the only person who believes Drivechains are a broken
idea, I'd suggest you take a step back and reconsider what you are saying here.
|
What responsibilities going forward do devs have with asic compatibility? Seems like a potential conflict of interest in some situations. |
Concern over 33 bytes of data being added per block for blind merged mining seems disingenuous to me. |
The reviewers of this and other PRs/BIPs also are in a position of high risk of conflict of interests. Every Dev wants to merge more needless op_code complexity to Bitcoin, 'world computer' alike, but no Dev is ever proposing to ALSO code the respective OPT-OUT script to Full Nodes Core Software, so that the only people unbiased in this mess, the node operators, could sovereignly (UN)support by themselves every new altcoin alike 'scaling' (altcoiners fav word) feature and non-bug fixing CHANGE to Bitcoin. Running an old node, less and less supported is NOT a decentralizing option either at some point, but much less a centralized Bitcoin filled with code complexity and consequently easier data dumping out of Bitcoin's sole goal (=ONE!) = tyranically (SN's terminology) tiny scope. |
Obviously we should not do things that are incompatible with existing ASICs without a very good reason. A feature that doesn't actually work is clearly not a good reason. |
Held my words here. A twitter space did happen end of last week between one of the PR author and myself, which was pleasant overall. I made it clear I didn’t have any strong thoughts on drivechain, though I took time to express 2 generic concerns I think affecting all bitcoin second-layers designed and developed for scalability or extend use-case expressivity. Lack of common metrics on L2s performance on multiple dimensions. E.g onboarding scaling (how many users can co-exist off-chain ?), transactional scaling (how many transfers can be performed off-chain per-on-chain transaction ?), fees cost scaling (what is the average / worst-case on-chain fees / off-chain fees on end-user ?), user computing ressources assumptions (e.g what is the average bandwidth cost ?). For now, it sounds to me people are comparing apples with oranges when talking about Bitcoin L2s. Partial risks and security analysis. Since side-chains have been proposed almost a decade ago, I think the community knowledge in term of L2 vector of attacks has growth a lot, especially from the deployment of Lightning. It sounds very plausible that new vectors e.g things like flood-and-loot, pinning, eclipse and time-dilation attacks, liquidity griefing are affecting all constructions in minor or major ways. E.g the 2014 sidechain paper made the observation that most users might use atomic swaps for coins transfers, a time-sensitive L2 protocol. My personal flavor in term of bitcoin consensus changes stay the same. Fix old consensus technical debt such as the timewarp attack, or the thundering herd issue affecting all time-sensitive second-layers. |
There hasn't been much activity lately and the patch still needs rebase. What is the status here?
|
Closing. The discussion here has run it's course, and it ultimately turned non-technical (this isn't the correct forum for that). |
This is a (rough draft) clean rewrite of BIP300 (Drivechain) consensus-level code.
Instead of a separate sidechain database (which may be prone to hard-to-review/test consistency issues), instead the unusable 8 MSB of the UTXO index are reserved for non-UTXO database entries, and the existing UTXO db and caching layer is shared. This can be refactored in the future, but I think it is the cleanest and most reviewable approach initially - open to other ideas, though. There's also some ugliness in the undo data to handle restoring the new data, but it's abstracted and shouldn't be too hard to reason about.
Using these new primitives, Drivechains can be reimplemented with a UTXO-like model. Note that there is zero activation logic in the current PR: the protocol changes are always active. Therefore, this will not work (at least not safely) on Bitcoin today, and cannot be deployed without significant additional changes to handle an activation.
A new
SERIALIZE_TRANSACTION_FOR_WEIGHT
serialization flag is also added, that is meant only for weight counting. This allows for adjusting weight (upward) to fit additional resource requirements by new functionality. In this case, several Drivechain "messages" are expected to have a larger burden than theirOP_RETURN
encoding would otherwise weigh. However, the specific adjustments are not implemented in this draft.As a consensus change, this can only be implemented with community support. Many people seem to have opinions, but please keep them to other forums. Despite providing and continuing this implementation, I myself do not thereby endorse or otherwise comment on the proposal itself.
Therefore, not looking for concept ACKs/NACKs (ie, about Drivechains), just Approach ACKs / constructive criticism (ie, about how I'm implementing it).