Zero Confirmation Forfeits (ZCF) #964

awemany · 2018-11-20T12:32:11Z

Hey guys,

here's my very preliminary code to do zero confirmation forfeits with Electron Cash. This should serve both as a test bed for development of the concept, as well as later on a reference implementation from which a specification can be derived.

I like to do the bottom-up approach of first writing code and the writing a specification that matches the code and all the pitfalls that were encountered writing it.

What kind-of works:

Transactions can be generated with a forfeit output. The forfeit output is currently set to 1.0 the send transaction amount.
Receiving transactions and extracting the forfeit amount, checking for enough
Display of forfeits in the transaction dialog, history list tab and addresses tab
Spending forward of the forfeits by assembling the right inputs to the the P2SH forfeit contract

There are lots of bugs still. Sometimes the value parser seems to miss forfeits. Sometimes I get bugs when starting up. It is all in a state of flux. I am publishing this PR early as @HostFat and some others were curious about this. That said, I tried to clean up my code as much as possible and separate it into a series of clean commits.

BIG FAT WARNING: This code is by no means production ready!! If you want to try it out, use test net or create a wallet with some miniscule amount of BCH in it. Losing money is still likely.

EDIT: For those curious to try this out, here's a walk through the functionality with pictures: https://github.com/awemany/electron-cash-zcf-demo/blob/master/README.md

cculianu · 2018-11-20T22:50:37Z

That was fast. You rock man. Thank you.

awemany · 2018-11-21T13:19:02Z

@cculianu: Thanks, but this is far from finished!

This is the code to generate the forfeit P2SH smart contract scripts.

Add parsing of generic P2SH contract addresses to the transaction code. Note that this still needs a check (maybe?) whether the previous transaction actually had a P2SH output. It is important to notice that a transaction's TXID is recalculated from these values. As unknown scriptSigs need to still produce the right serialization, the serialization code has likewise been changed to always use an input's scriptSig, if available.

This adds support for handling forfeit addresses to the wallet. Two new class members are introduced: forfeit_map: This maps forfeit addresses to their underlying P2PKH address (for which the public key also matches the forfeit address if using the correct P2SH script for scriptSig generation). forfeit_scripts: This maps forfeit addresses to their corresponding P2SH scripts that allow to spend the forfeit addresses as normal P2PKH (or by two sigs in case a miner detects a double-spend). Both of these objects are persisted to the wallet JSON file for easier handling and to cache the transaction parsing results. It also adds support to the various accessor function to more properly represent forfeit addresses. It still misses the transaction generating code as well as the code to extract forfeits from incoming transactions that match the template forfeit P2SH code. This will come in an upcoming commit.

This checks incoming transactions in add_transaction() for forfeit outputs and, if such outputs are found, adds them to the tracking data structures self.forfeit_map and self.forfeit_scripts.

If the configuration variable use_forfeits is set to true, this will generate transaction that have a "ZCF\x00" OP_RETURN marker indicating the use of forfeits as well as having the special forfeit smart contract output to a corresponding P2SH address. The P2SH address is deterministically generated from the transaction's inputs and the change address that is alsoused for the "forfeit change". This means that it is possible to recreate the P2SH contents just by trying reconstruction of all available outputs and checking for a matching P2SH hash.

Note that this comes with a considerable rework and simplification of the listing logic. This might miss some subtlety regarding the address listing and therefore needs careful review.

This shows a green "F" for unconfirmed transactions that have enough forfeit amount for the given received value. FIXME: The amount is currently hardcoded to be a minimum of 1.0 the received value. It probably makes sense to add a configuration dialog for this.

FIXME: This directly changes the configuration variable. Is this an acceptable way to do it?

For signalling problems when generating forfeit transactions. Also remove a superfluous return after an exception is raised.

... instead of throwing an uncontrolled exception later on.

- this show forfeits already when creating a new transaction - it also shows one's owns and other's forfeit contract P2SH addresses in different colors (light blue for own forfeit outputs, pink for those from others)

This is necessary as to not forget about the forfeit addresses so that the balance is shown correctly.

awemany · 2018-11-21T17:30:52Z

Ok, updates:

Important fixes. The correct forfeit and amount display seems to be a lot more stable now. Still, wouldn't be surprised if there's still bugs lurking along these lines. For any new forfeit detected, save_addresses() is called, which might be overdoing it but it seems to work.
wallet upgrade from non-forfeit to forfeit-mode supported
fixed a stupid bug that actually prevented proper display of forfeit-enabled transactions with the special "green F marker"
both own forfeit contracts (back as 'forfeit change' to oneself) as well as to others are displayed in the transaction dialog now, also when creating a fresh TXN. Own and other forfeits are displayed in different colors (lightblue, pink)
Better, though improvable parsing of forfeit inputs as P2SH. Note that Electrum/EC has some weird assumptions internally so far that P2SH is always multisig
display error messages instead of 'not enough funds' for the various ways forfeit creation might fail

To have it displayed correctly on the preview tab. FIXME: This might create some noise when aborting the transaction generation and using different inputs down the road.

…values

awemany · 2018-11-27T16:26:27Z

Updated with a configuration dialog for the send and accepted receive forfeit fractions. Needs more testing.

gasull

Three tiny improvements with intention of being "right work".

gasull · 2019-08-02T11:24:51Z

gui/qt/address_list.py

+        for addr_list, name in [
+            (self.wallet.get_receiving_addresses(), _("Receiving")),
+            (self.wallet.get_change_addresses(), _("Change")),
+            (self.wallet.get_forfeit_addresses(), _("Forfeit"))]:


For readability, and since the iterating list is never modified, I'd suggest using a tuple and defining it before the loop:

addresses_to_name_pairs = ( (self.wallet.get_receiving_addresses(), _("Receiving")), (self.wallet.get_change_addresses(), _("Change")), (self.wallet.get_forfeit_addresses(), _("Forfeit")), ) for addr_list, name in addresses_to_name_pairs:

gasull · 2019-08-02T11:32:02Z

gui/qt/main_window.py

@@ -3084,12 +3093,57 @@ def on_fiat_address(checked):
        fiat_widgets.append((QLabel(_('Show Fiat balance for addresses')), fiat_address_checkbox))
        fiat_widgets.append((QLabel(_('Source')), ex_combo))

+


Nitpicking: why the extra blank line here?

gasull · 2019-08-02T11:32:08Z

gui/qt/address_list.py

+                continue
+            seq_item = QTreeWidgetItem( [ name, '', '', '', '', ''] )
+            account_item.addChild(seq_item)
+            if not had_item_count: # first time we create this widget, auto-expand the default address list


PEP 8 nitpicking:

Inline comments should be separated by at least two spaces from the statement.

awemany force-pushed the zero-conf-forfeits branch from 1de5349 to 58ce9b3 Compare November 20, 2018 12:38

awemany added 25 commits November 21, 2018 18:12

Add CDS/-V to address/OpCodes

cb3d3d8

Add *~ backup files to .gitignore

adfacb8

Bug fix, there's no True arg to .to_ui_string()

952c9d6

Add an icon for unconfirmed transactions with forfeit

04d7f01

Zero conf forfeit specific infrastructure code

91fc92e

This is the code to generate the forfeit P2SH smart contract scripts.

transaction/input_script(): Support for 'forfeit_p2kh'

d244d76

transaction/get_preimage_script: Support for forfeit scripts

b2b2e92

wallet: Add warning wrt. use of is_beyond_limit()

e7970ba

wallet: Extract forfeit data from incoming transactions

dd135a1

This checks incoming transactions in add_transaction() for forfeit outputs and, if such outputs are found, adds them to the tracking data structures self.forfeit_map and self.forfeit_scripts.

transaction_dialog: Mark known forfeit outputs in light blue

cc4e087

address_list: Also show forfeit outputs

296feb4

Note that this comes with a considerable rework and simplification of the listing logic. This might miss some subtlety regarding the address listing and therefore needs careful review.

main_window: Support for toggling the use of forfeits

7c16746

FIXME: This directly changes the configuration variable. Is this an acceptable way to do it?

wallet: Use BaseException() instead of NotEnoughFunds()

1fc08d8

For signalling problems when generating forfeit transactions. Also remove a superfluous return after an exception is raised.

wallet: Extra check that forfeit inputs are not used twice

b500e1d

wallet: Reload history if there's no forfeit_map in the wallet yet

9fc6476

wallet: Guard against forfeit underlying address not in change_addresses

9c5cebc

wallet: Warn for not enough input for forfeit txn

82b17cc

... instead of throwing an uncontrolled exception later on.

wallet: Some ZCF debug value tweaks

e5d82ef

transaction_dialog: Better display of forfeits

8e30d42

- this show forfeits already when creating a new transaction - it also shows one's owns and other's forfeit contract P2SH addresses in different colors (light blue for own forfeit outputs, pink for those from others)

wallet: Add forfeit address to query list and save address list

fa22d4d

This is necessary as to not forget about the forfeit addresses so that the balance is shown correctly.

wallet: Only add own forfeit addresses...

380d725

awemany force-pushed the zero-conf-forfeits branch from 58ce9b3 to 380d725 Compare November 21, 2018 17:25

awemany added 2 commits November 21, 2018 20:41

wallet: Add forfeit transaction when creating

41fbbdc

To have it displayed correctly on the preview tab. FIXME: This might create some noise when aborting the transaction generation and using different inputs down the road.

use_forfeits needs to be boolean... Ooops :)

4bb4518

cculianu added the Do Not Merge (Yet!) Pull requests that we intend to merge in the future but that should not yet be merged. label Nov 25, 2018

awemany added 2 commits November 27, 2018 17:16

Tab for ZCF configuration

36db845

Forfeits: Consider configured minimum receive and send multiplicator …

dbd4457

…values

gasull reviewed Aug 2, 2019

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Zero Confirmation Forfeits (ZCF) #964

Zero Confirmation Forfeits (ZCF) #964

awemany commented Nov 20, 2018 •

edited

Loading

cculianu commented Nov 20, 2018

awemany commented Nov 21, 2018

awemany commented Nov 21, 2018

awemany commented Nov 27, 2018

gasull left a comment

gasull Aug 2, 2019 •

edited

Loading

gasull Aug 2, 2019

gasull Aug 2, 2019

		@@ -3084,12 +3093,57 @@ def on_fiat_address(checked):
		fiat_widgets.append((QLabel(_('Show Fiat balance for addresses')), fiat_address_checkbox))
		fiat_widgets.append((QLabel(_('Source')), ex_combo))

Zero Confirmation Forfeits (ZCF) #964

Are you sure you want to change the base?

Zero Confirmation Forfeits (ZCF) #964

Conversation

awemany commented Nov 20, 2018 • edited Loading

cculianu commented Nov 20, 2018

awemany commented Nov 21, 2018

awemany commented Nov 21, 2018

awemany commented Nov 27, 2018

gasull left a comment

Choose a reason for hiding this comment

gasull Aug 2, 2019 • edited Loading

Choose a reason for hiding this comment

gasull Aug 2, 2019

Choose a reason for hiding this comment

gasull Aug 2, 2019

Choose a reason for hiding this comment

awemany commented Nov 20, 2018 •

edited

Loading

gasull Aug 2, 2019 •

edited

Loading