ELIP 202: Implementation of optional sidechain peg-in subsidy and minimum peg-in amount#1499
Conversation
|
minor nit: mix of "pegin" and "peg-in" in documentation and error messages. |
|
Currently I run a pruned Elements node on liquidv1 with the default I don't like the idea of the main chain Bitcoin transaction fee influencing the amount getting burned on liquidv1 during peg-in (after being enabled, of course, but adding the possibility now into the code just begs for enabling it eventually). Concept ACK now as I did not know the context. Edit: I see the reasoning now thanks to explanation from Pablo. |
|
Tested ACK 7353687 |
|
Rebased to remove the fixup commit - sorry to invalidate your ACK @tomt1664 |
In order for a claim to work, the tx needs to be at least 100 blocks deep, so that is unaffected. |
Do you mean with validatepegin=0? Even so, you can still compose the peg-in transaction with subsidy correctly by passing the parent (bitcoin) transaction feerate to
As the Liquid Federation drives the tech roadmap for liquidv1, it's their prerogative should they want this enabled to subsidise or discourage low value peg-ins that they must pay for on the mainchain to sweep or spend.
Noted.
It may be worth reading it again. |
In CreatePeginWitnessInner, the MerkleBlock is always serialized without witness: PROTOCOL_VERSION | SERIALIZE_TRANSACTION_NO_WITNESS In DecomposePeginWitness before this change, the MerkleBlock was deserialized with witness: PROTOCOL_VERSION This was only noticed as an issue in the pegin subsidy implementation, in a failure in the feature_dynafed functional test. In the test_transition_mempool_eject test case, the Merkle block proof is coming from the same chain where we are creating a pegin. See the comment: "hack: since we're not validating peg-ins in parent chain, just make both the funding and claim tx on same chain (printing money)" I haven't investigated enough to explain why this causes a deserialization failure in this specific case, but presumably this change is correct since we're always serializing without witness. Before this DecomposePeginWitness was only used in src/psbt.cpp
|
Rebased against master |
Thank you. I have changed my mind already.
I did. But still I would suggest adding some context (maybe a link if it was discussed anywhere). I am not in any office, nor a Telegram channel now, to hear any first-hand gossip from people. And it could be interesting in the future to have an idea about the things happening at the time. Of course no one wants to add wood to fire but misunderstandings like this can happen exactly when there is missing context I'd say. Thank you for resolution, @delta1 ! |
|
Runnung a18a88b with one-liner here-uncommitted change of Running ACK a18a88b |
|
Tested ACK a18a88b |
fa01ad2 Update manpages (Pablo Greco) cc2dd3c Bump version to 23.3.3-rc2 (Pablo Greco) 87b72b8 Revert "Merge #1499: ELIP 202: Implementation of optional sidechain peg-in subsidy and minimum peg-in amount" (Pablo Greco) 56eb713 Log status of ELIP 203 (Pablo Greco) d805236 liquidv1: allow enabling -acceptunlimitedissuances (Pablo Greco) bd59321 contrib: upstream my merge-prs.sh changes (Byron Hambly) a18a88b grammar: prefer peg-in to pegin in messages (Byron Hambly) 90b2b3b test: add pegin subsidy functional test (Byron Hambly) 186bb25 validation: check for peg-in subsidy and minimum (Byron Hambly) 51c89c6 subsidy: implementation for claimpegin, createrawpegin, and RPCs (Byron Hambly) 94cde59 subsidy: add chainparams and init (Byron Hambly) f3b63f4 DecomposePeginWitness: fix deserialization flags for MerkleBlock proof (Byron Hambly) Pull request description: Updates master to c990492, which adds support for enabling ELIP 203 in liquidv1, but with ELIP 202 reverted. Also and bumps version/manpages ACKs for top commit: delta1: ACK fa01ad2; tested locally, diff as expected tomt1664: Tested ACK fa01ad2 Tree-SHA512: b84da4948d72a638a107868a89676387ec4154e53f3e3c8b937db4cba022819945d49e3f9630c3d6c4574fe32ae8931cc8088374a02229ea027c9f50c487008c
Implementation for ELIP 202