2WP: Allow other elements chains as parent chains for pegs by jtimon · Pull Request #362 · ElementsProject/elements

jtimon · 2018-05-25T17:39:21Z

Fix #360 (doesn't really fix it as noted in #362 (comment) , but it is still useful for testing)

Dependencies:

Refactor: Validation: Make parts of IsValidPeginWitness templated Refactor: Validation: Make parts of IsValidPeginWitness templated #370

jtimon · 2018-06-04T20:51:16Z

Rebased after #363 was merged, which was a requirement for this (or some other way to do the same).
Also fixed a couple of bugs and greatly improved the testing. The new test is mostly a copy of pegging.py but adapted for the test framework and with some parts commented that are not properly adapted yet.
Ideally in the future we can get rid of pegging.py by offering an option to force a pow chain as parent chain by providing a path to a binary like we do in pegging.py.

jtimon · 2018-06-05T02:40:41Z

Removed the 'WIP' because now it seems to pass most of the tests.
This doesn't need to replace pegging.py, review and testing should focus on making sure the pervious default behavior is not broken in any way.

instagibbs · 2018-06-05T20:02:53Z

Will review

instagibbs · 2018-06-06T17:49:53Z

src/init.cpp

        strUsage += HelpMessageOpt("-parentpubkeyprefix", strprintf(_("The byte prefix, in decimal, of the parent chain's base58 pubkey address. (default: %d)"), 111));
        strUsage += HelpMessageOpt("-parentscriptprefix", strprintf(_("The byte prefix, in decimal, of the parent chain's base58 script address. (default: %d)"), 196));
-
+        strUsage += HelpMessageOpt("-con_parent_chain_has_pow", "Whether parent chain uses pow or signed blocks default: 1");


put default in parenthesis

instagibbs · 2018-06-06T17:51:48Z

src/validation.cpp

+}
+
+template<typename T>
+static bool CheckPegoutTx(const std::vector<unsigned char>& tx_data, T& pegtx, const COutPoint& prevout, const CAmount claim_amount, const CScript& claim_script)


CheckPeginTx?

instagibbs · 2018-06-06T17:56:19Z

src/validation.cpp

+            return false;
        }
-        if (merkle_block.txn.ExtractMatches(txHashes, txIndices) != merkle_block.header.hashMerkleRoot || txHashes.size() != 1) {
+        // TODO do this inside to prever DoS ?


instagibbs · 2018-06-06T18:04:20Z

concept ACK. Going to have to carefully comb over these changes.

instagibbs · 2018-06-06T18:04:46Z

Could we split up these changes into basic refactors, then consensus changes?

jtimon · 2018-06-14T17:18:46Z

Uncommented more tests.

stevenroose · 2018-08-03T16:31:23Z

qa/rpc-tests/feature_fedpeg.py

+        proof = parent.gettxoutproof([txid1])
+
+        raw = parent.getrawtransaction(txid1)
+        import json


That's a strange place to import json..

stevenroose · 2018-08-06T15:16:42Z

qa/rpc-tests/feature_fedpeg.py

+            print(e.error["message"])
+            assert("Given or recovered script is not a witness program." in e.error["message"])
+
+        # # Should fail due to non-matching wallet address


Is this here because it should fail due to this if it would not fail due to non-witness (clause above)?

This is very good for clarity, but it doesn't really test if it would fail in that case..

That's simply the same test as in https://github.com/ElementsProject/elements/blob/elements-0.14.1/qa/rpc-tests/pegging.py#L233 but commented because I didn't manage to make it work, note here there's a similar one above "Should fail due to non-witness", perhaps that's enough.
Perhaps I should just remove that commented code, but then I will forget about it.
@instagibbs thoughts?

was this functionality supposed to be working?

instagibbs · 2018-08-07T15:18:07Z

How much extra work is it to do this properly and check for the correct asset id? Shouldn't it just be a single check?

jtimon · 2018-08-10T04:19:43Z

Fixed import nit, rebased (was clean).

@instagibbs Added a wip commit doing the check, but the tests fail, unless I'm misunderstanding something, because the asset id in the pegin tx from the parent chain is not explicit but confidential.
If there's an easy way to "always do explicit assets by default" or something similar, I guess this could work.

instagibbs · 2018-08-10T13:07:48Z

The output shouldn't be confidential though? The mainchain_address is unconfidential, yes?

jtimon · 2018-08-10T21:50:26Z

Right, sorry, it was just a bug that confused me. It seems to work now. Let me know if you prefer me to squash, rename the commit or something else. I would squash, I think.

instagibbs · 2018-08-16T17:09:14Z

qa/rpc-tests/feature_fedpeg.py

        self.parentgenesisblockhash = self.nodes[0].getblockhash(0)
        print('parentgenesisblockhash', self.parentgenesisblockhash)
+        parent_pegged_asset = self.nodes[0].getsidechaininfo()['pegged_asset']
+        print('parent_pegged_asset', parent_pegged_asset)


errant print?

This allows other elements chains as parent chains for pegs, for testing purposes for now it also assumes that the parent chain has CT/CA tx format, which is orthogonal to the parent chain having pow or signed blocks It would be nice to also test bitcoin-like chains with signed blocks instead of pow by adding another parameter -con_parent_chain_has_CT or similar and decouple the logic Introduce -con_parent_pegged_asset option to check the asset in the peg-in txs from the parent chains. CT: Introduce GetAmountFromParentChainPegin for CT txs RPC: Adapt rpcwallet to initial -con_parent_chain_has_pow Introduce CheckProofGeneric Introduce pow::CheckProofParent

instagibbs · 2018-09-05T16:44:52Z

src/wallet/rpcwallet.cpp

        throw JSONRPCError(RPC_TYPE_ERROR, "The included txoutproof is malformed. Are you sure that is the whole string?");
    }

-    if (!ssTxOutProof.empty() || !CheckBitcoinProof(merkleBlock.header.GetHash(), merkleBlock.header.nBits))


why are we dropping this check?

It is moved to https://github.com/ElementsProject/elements/pull/362/files/2de576525b4d3ccc7d87d8d27fa038a5632cf84e#diff-df7d84ff2f53fcb2a0dc15a3a51e55ceR3719

instagibbs · 2018-09-05T16:46:32Z

utACK 3f62b32 other than single question

… chains for pegs 3f62b32 QA: Test 2wp using the test framework (Jorge Timón) 2de5765 2WP: QA: Introduce -con_parent_chain_signblockscript (Jorge Timón) 781953b QA: Optionally allow other chains (Jorge Timón)

7c33e3a qt: Add missing mnemonics in menu bar options (Shashwat) Pull request description: Since ElementsProject#362 we have defaulted to add mnemonic shortcuts for the context menus. The Window -> Minimize option and File -> Load PSBT from clipboard were hitherto missing a mnemonic shortcut. This PR adds mnemonic shortcuts for them Changes introduced in this PR: | Master | PR | | ----------| ---- | | ![Screenshot from 2021-08-23 23-10-07](https://user-images.githubusercontent.com/85434418/130494098-c65ec9da-c3f1-4243-9b3d-0c87cb677825.png) | ![Screenshot from 2021-08-23 23-08-41](https://user-images.githubusercontent.com/85434418/130494083-849ffd14-05e9-4a6d-bdc3-b3e55b8a8861.png)| |![Screenshot from 2021-08-23 23-10-21](https://user-images.githubusercontent.com/85434418/130494233-1b2e8838-c5d4-48a8-9abf-a4acc8d6146c.png)|![Screenshot from 2021-08-23 23-09-00](https://user-images.githubusercontent.com/85434418/130494181-dcdbf119-a2c6-469d-a6c9-3021506ab40b.png)| ACKs for top commit: jarolrod: tACK 7c33e3a hebasto: ACK 7c33e3a, tested on Linux Mint 20.2 (Qt 5.12.8). Tree-SHA512: 32f201ae7716b19ca123856292f8bfa0d805f6c7271ac1b5e08eff6b95017443754c8a76e8396ccca1869a57384e11016cbd99d63ccdd2fae6da4eaf3ae32298

jtimon force-pushed the e14-parent-sigchain branch from 30ec4dd to 2126521 Compare May 27, 2018 19:33

jtimon mentioned this pull request May 31, 2018

allow setting parent chain address prefix in configuration file #363

Merged

jtimon force-pushed the e14-parent-sigchain branch 3 times, most recently from 773ab52 to e756199 Compare June 4, 2018 20:40

jtimon force-pushed the e14-parent-sigchain branch from e756199 to cbf9912 Compare June 4, 2018 21:17

jtimon mentioned this pull request Jun 5, 2018

RPC: Stop providing useless fake pow data #366

Closed

1 task

jtimon force-pushed the e14-parent-sigchain branch from cbf9912 to 19ecf0c Compare June 5, 2018 02:36

jtimon changed the title ~~WIP: 2WP: Allow signed blocks chains as parent chains for pegs~~ 2WP: Allow signed blocks chains as parent chains for pegs Jun 5, 2018

instagibbs reviewed Jun 6, 2018

View reviewed changes

jtimon force-pushed the e14-parent-sigchain branch 2 times, most recently from 2e077cc to 2f0f2fc Compare June 12, 2018 21:17

jtimon mentioned this pull request Jun 12, 2018

Refactor: Validation: Make parts of IsValidPeginWitness templated #370

Merged

jtimon force-pushed the e14-parent-sigchain branch 10 times, most recently from 50338cc to 6de5683 Compare June 14, 2018 17:18

jtimon force-pushed the e14-parent-sigchain branch from 6de5683 to 4118621 Compare June 14, 2018 17:35

jtimon force-pushed the e14-parent-sigchain branch 3 times, most recently from 5cae62a to 3f10c98 Compare July 22, 2018 06:29

jtimon mentioned this pull request Jul 24, 2018

QA: Chainparams: Run functional tests with custom chain #274

Closed

3 tasks

stevenroose reviewed Aug 3, 2018

View reviewed changes

stevenroose reviewed Aug 6, 2018

View reviewed changes

QA: Optionally allow other chains

781953b

jtimon force-pushed the e14-parent-sigchain branch 2 times, most recently from dbcc884 to 502357a Compare August 10, 2018 04:16

jtimon force-pushed the e14-parent-sigchain branch 2 times, most recently from 58b09f9 to abc8305 Compare August 10, 2018 21:44

jtimon changed the title ~~2WP: Allow other elements chains as parent chains for pegs (for testing only)~~ 2WP: Allow other elements chains as parent chains for pegs Aug 10, 2018

jtimon mentioned this pull request Aug 16, 2018

2WP: Verify number of transactions in SPV proof #400

Merged

instagibbs reviewed Aug 16, 2018

View reviewed changes

jtimon added 2 commits August 16, 2018 19:29

QA: Test 2wp using the test framework

3f62b32

jtimon force-pushed the e14-parent-sigchain branch from abc8305 to 3f62b32 Compare August 16, 2018 17:31

instagibbs reviewed Sep 5, 2018

View reviewed changes

instagibbs approved these changes Sep 6, 2018

View reviewed changes

jtimon referenced this pull request in kallewoof/bitcoin Sep 6, 2018

Signet implementation

e416403

jtimon merged commit 3f62b32 into ElementsProject:elements-0.14.1 Sep 6, 2018

jtimon deleted the e14-parent-sigchain branch September 6, 2018 22:18

jtimon mentioned this pull request Sep 21, 2018

Integrate pegging.py in qa/rpc-tests/feature_fedpeg.py #127

Closed

Comments

Conversation

jtimon commented May 25, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jtimon commented Jun 4, 2018

Uh oh!

jtimon commented Jun 5, 2018

Uh oh!

instagibbs commented Jun 5, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

instagibbs commented Jun 6, 2018

Uh oh!

instagibbs commented Jun 6, 2018

Uh oh!

jtimon commented Jun 14, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

instagibbs commented Aug 7, 2018

Uh oh!

jtimon commented Aug 10, 2018

Uh oh!

instagibbs commented Aug 10, 2018

Uh oh!

jtimon commented Aug 10, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

instagibbs commented Sep 5, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jtimon commented May 25, 2018 •

edited

Loading