Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[r2r] fix spv proof validation #1268

Merged
merged 7 commits into from
Apr 29, 2022
Merged

[r2r] fix spv proof validation #1268

merged 7 commits into from
Apr 29, 2022

Conversation

onur-ozkan
Copy link
Member

@onur-ozkan onur-ozkan commented Apr 27, 2022 •
edited
Loading

  • Start spv proof validation conditionally depended on the parameter enable_spv_proof in coins configuration.

Fixes #1239

2022-04-27_17-23
2022-04-27_17-21

@onur-ozkan
save development state
b02e02e 
Signed-off-by: ozkanonur <work@onurozkan.dev>
@onur-ozkan
fix clippy error
b0043d3 
Signed-off-by: ozkanonur <work@onurozkan.dev>
@onur-ozkan
Copy link
Member Author

onur-ozkan commented Apr 27, 2022 •
edited
Loading

Function validate_spv_proof now will call a function try_spv_proof_until in order to get merkle_branch and block_header. And try_spv_proof_until is basically an error-prone loop, executes the inside algorithm every 10 seconds until reaching the deadline or getting the needed values from electrum RPC calls. In case if the function reaches the deadline, it will return SPVError::Timeout.

So, for swap operations, I used the same deadline that been used for wait_for_confirmations. For tests I used 30 seconds, and for SLP tokens I used 60 seconds deadline. But, I am not sure about the SLP tokens. 60 seconds is basically 6x retry in case of error in try_spv_proof_until function. Please let me know if I should update the deadline for SLP tokens. @artemii235

@onur-ozkan onur-ozkan changed the title Fix spv proof validation [r2r] fix spv proof validation Apr 27, 2022
@artemii235 artemii235 requested a review from cipig April 27, 2022 14:49
@artemii235
Copy link
Member

@ozkanonur Great work :)
I will review this after @cipig confirmation that the problem is fixed.

@onur-ozkan
log blockchain_transaction_get_merkle errors on debug level
a6c9892 
Signed-off-by: Onur Özkan <work@onurozkan.dev>
cipig
cipig previously approved these changes Apr 27, 2022
View reviewed changes
Copy link
Member

@cipig cipig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

works fine now, saw output like this, also 2 times, all swaps finished successfully

27 16:28:33, coins:utxo_common:2995] ERROR InvalidHeight
27 16:28:33, coins:utxo_common:3017] ERROR Failed spv proof validation for transaction 41a25046f02906abb376717d20beeef64abb525e6b4e5b7630ef58bdb2241c37, retrying in 10 seconds.

@onur-ozkan
update retry pool flow
32f4b73 
Signed-off-by: ozkanonur <work@onurozkan.dev>
@onur-ozkan onur-ozkan changed the title [r2r] fix spv proof validation [wip] fix spv proof validation Apr 28, 2022
@onur-ozkan
start spv validation conditionally
f8eb11f 
Signed-off-by: ozkanonur <work@onurozkan.dev>
@onur-ozkan onur-ozkan changed the title [wip] fix spv proof validation [r2r] fix spv proof validation Apr 28, 2022
@onur-ozkan
fix testing
cc88b1b 
Signed-off-by: ozkanonur <work@onurozkan.dev>
@onur-ozkan
update deadline for validate_spv_proof in testing
dcb7d8e 
Signed-off-by: ozkanonur <work@onurozkan.dev>
cipig
cipig approved these changes Apr 28, 2022
View reviewed changes
Copy link
Member

@cipig cipig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

spv proof can be enabled in coins file: KomodoPlatform/coins@9e7cd41

swaps working fine for the coins with spv proof on including retries

28 17:55:27, coins:utxo_common:2994] ERROR InvalidHeight for tx Transaction { version: 1, n_time: None, overwintered: false, version_group_id: 0, inputs: [TransactionInput { previous_output: OutPoint { hash: e7dfc36a8760baf46ba78fec121804115a6a08e21fd4beb616a7ee5e6b729fe3, index: 1 }, script_sig: 4830450221009b242e495d034fa2f157488e27a08019417a09342ff5590768ac148d0a6697100220608ea23ae98e5596df5797220676be7f6dae996b0480847dbf726da99a58c07a0121039ef1b42c635c32440099910bbe1c5e8b0c9373274c3f21cf1003750fc88d3499, sequence: 4294967295, script_witness: [] }, TransactionInput { previous_output: OutPoint { hash: 79bb24bb88a30e4b992eeaa80aab34a2199c4808d567317cf3c05c2a8da11950, index: 0 }, script_sig: 473044022024d396988e2d6b6df9e89892b7266fc7b5244eb32e3638ed86864f825bd2f69d022037c7131d5cdf4eef51f01fb0e7d3f7eedb236ea3abc2265ea54e0a12a637c9c70121039ef1b42c635c32440099910bbe1c5e8b0c9373274c3f21cf1003750fc88d3499, sequence: 4294967295, script_witness: [] }], outputs: [TransactionOutput { value: 18251356, script_pubkey: a914313d3622a48198595ea230a65dc36c255560a14487 }, TransactionOutput { value: 0, script_pubkey: 6a14ebf00744edff7069c87753c105fd4c35f94510f7 }], lock_time: 1651168015, expiry_height: 0, shielded_spends: [], shielded_outputs: [], join_splits: [], value_balance: 0, join_split_pubkey: 0000000000000000000000000000000000000000000000000000000000000000, join_split_sig: 00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000, binding_sig: 00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000, zcash: false, str_d_zeel: None, tx_hash_algo: DSHA256 }
28 17:55:27, coins:utxo_common:3035] ERROR Failed spv proof validation for transaction 5b9f4779291cb20059479c5f88b184dcbdc17effd2cfae7796742fb8eecd2818, retrying in 10 seconds.

and for the ones that need to have it off like LBC and QTUM

saw this output during a KMD/QTUM swap on maker side (maker_coin = QTUM)

+--- 28 17:03:21 -------
| (0:33) [swap uuid=137ddfb4-c8a7-4e59-986d-127bb1482b1f] Maker payment sent...
28 17:03:21, coins:rpc_clients:148] INFO Waiting for tx 8d88508e0a68f59336fcd06de155b23c647c10e93f3ec47abbc83d2418a6600a confirmations, now 0, required 1, requires_notarization false

28 17:03:26, utxo_common:2763] utxo_common:2733] Warning, actual block_count 1826307 less than cached tx_height 1826308 of 8d88508e0a68f59336fcd06de155b23c647c10e93f3ec47abbc83d2418a6600a
28 17:03:26, coins:utxo_common:2876] WARN Attempt N0: spendable balance 143.57607083 greater than total balance 128.57207083

28 17:03:27, utxo_common:2763] utxo_common:2733] Warning, actual block_count 1826307 less than cached tx_height 1826308 of 8d88508e0a68f59336fcd06de155b23c647c10e93f3ec47abbc83d2418a6600a
28 17:03:27, coins:utxo_common:2876] WARN Attempt N1: spendable balance 143.57607083 greater than total balance 128.57207083

28 17:03:27, utxo_common:2763] utxo_common:2733] Warning, actual block_count 1826307 less than cached tx_height 1826308 of 8d88508e0a68f59336fcd06de155b23c647c10e93f3ec47abbc83d2418a6600a
28 17:03:27, coins:utxo_common:2876] WARN Attempt N0: spendable balance 143.57607083 greater than total balance 128.57207083

28 17:03:27, utxo_common:2763] utxo_common:2733] Warning, actual block_count 1826307 less than cached tx_height 1826308 of 8d88508e0a68f59336fcd06de155b23c647c10e93f3ec47abbc83d2418a6600a
28 17:03:27, utxo_common:2763] utxo_common:2733] Warning, actual block_count 1826307 less than cached tx_height 1826308 of 8d88508e0a68f59336fcd06de155b23c647c10e93f3ec47abbc83d2418a6600a
28 17:03:27, coins:utxo_common:2876] WARN Attempt N1: spendable balance 143.57607083 greater than total balance 128.57207083

28 17:03:27, utxo_common:2763] utxo_common:2733] Warning, actual block_count 1826307 less than cached tx_height 1826308 of 8d88508e0a68f59336fcd06de155b23c647c10e93f3ec47abbc83d2418a6600a
28 17:03:27, rpc:282] ERROR RPC error response: rpc:208] dispatcher_legacy:160] lp_ordermatch:4193] check_balance:28] utxo_common:2873] Internal error: Spendable balance 143.57607083 greater than total balance 128.57207083

28 17:03:37, maker_swap:755] Taker payment tx f03f0a9d69b66bab0d5df83b9d378b8b25992059d94de0f089a4c9da810c68ec
28 17:03:37, coins:rpc_clients:148] INFO Waiting for tx f03f0a9d69b66bab0d5df83b9d378b8b25992059d94de0f089a4c9da810c68ec confirmations, now 0, required 4, requires_notarization false
+--- 28 17:03:38 -------

idk what it means, but that swap also worked fine

@onur-ozkan
Copy link
Member Author

spv proof can be enabled in coins file: KomodoPlatform/coins@9e7cd41

swaps working fine for the coins with spv proof on including retries

28 17:55:27, coins:utxo_common:2994] ERROR InvalidHeight for tx Transaction { version: 1, n_time: None, overwintered: false, version_group_id: 0, inputs: [TransactionInput { previous_output: OutPoint { hash: e7dfc36a8760baf46ba78fec121804115a6a08e21fd4beb616a7ee5e6b729fe3, index: 1 }, script_sig: 4830450221009b242e495d034fa2f157488e27a08019417a09342ff5590768ac148d0a6697100220608ea23ae98e5596df5797220676be7f6dae996b0480847dbf726da99a58c07a0121039ef1b42c635c32440099910bbe1c5e8b0c9373274c3f21cf1003750fc88d3499, sequence: 4294967295, script_witness: [] }, TransactionInput { previous_output: OutPoint { hash: 79bb24bb88a30e4b992eeaa80aab34a2199c4808d567317cf3c05c2a8da11950, index: 0 }, script_sig: 473044022024d396988e2d6b6df9e89892b7266fc7b5244eb32e3638ed86864f825bd2f69d022037c7131d5cdf4eef51f01fb0e7d3f7eedb236ea3abc2265ea54e0a12a637c9c70121039ef1b42c635c32440099910bbe1c5e8b0c9373274c3f21cf1003750fc88d3499, sequence: 4294967295, script_witness: [] }], outputs: [TransactionOutput { value: 18251356, script_pubkey: a914313d3622a48198595ea230a65dc36c255560a14487 }, TransactionOutput { value: 0, script_pubkey: 6a14ebf00744edff7069c87753c105fd4c35f94510f7 }], lock_time: 1651168015, expiry_height: 0, shielded_spends: [], shielded_outputs: [], join_splits: [], value_balance: 0, join_split_pubkey: 0000000000000000000000000000000000000000000000000000000000000000, join_split_sig: 00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000, binding_sig: 00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000, zcash: false, str_d_zeel: None, tx_hash_algo: DSHA256 }
28 17:55:27, coins:utxo_common:3035] ERROR Failed spv proof validation for transaction 5b9f4779291cb20059479c5f88b184dcbdc17effd2cfae7796742fb8eecd2818, retrying in 10 seconds.

and for the ones that need to have it off like LBC and QTUM

saw this output during a KMD/QTUM swap on maker side (maker_coin = QTUM)

+--- 28 17:03:21 -------
| (0:33) [swap uuid=137ddfb4-c8a7-4e59-986d-127bb1482b1f] Maker payment sent...
28 17:03:21, coins:rpc_clients:148] INFO Waiting for tx 8d88508e0a68f59336fcd06de155b23c647c10e93f3ec47abbc83d2418a6600a confirmations, now 0, required 1, requires_notarization false

28 17:03:26, utxo_common:2763] utxo_common:2733] Warning, actual block_count 1826307 less than cached tx_height 1826308 of 8d88508e0a68f59336fcd06de155b23c647c10e93f3ec47abbc83d2418a6600a
28 17:03:26, coins:utxo_common:2876] WARN Attempt N0: spendable balance 143.57607083 greater than total balance 128.57207083

28 17:03:27, utxo_common:2763] utxo_common:2733] Warning, actual block_count 1826307 less than cached tx_height 1826308 of 8d88508e0a68f59336fcd06de155b23c647c10e93f3ec47abbc83d2418a6600a
28 17:03:27, coins:utxo_common:2876] WARN Attempt N1: spendable balance 143.57607083 greater than total balance 128.57207083

28 17:03:27, utxo_common:2763] utxo_common:2733] Warning, actual block_count 1826307 less than cached tx_height 1826308 of 8d88508e0a68f59336fcd06de155b23c647c10e93f3ec47abbc83d2418a6600a
28 17:03:27, coins:utxo_common:2876] WARN Attempt N0: spendable balance 143.57607083 greater than total balance 128.57207083

28 17:03:27, utxo_common:2763] utxo_common:2733] Warning, actual block_count 1826307 less than cached tx_height 1826308 of 8d88508e0a68f59336fcd06de155b23c647c10e93f3ec47abbc83d2418a6600a
28 17:03:27, utxo_common:2763] utxo_common:2733] Warning, actual block_count 1826307 less than cached tx_height 1826308 of 8d88508e0a68f59336fcd06de155b23c647c10e93f3ec47abbc83d2418a6600a
28 17:03:27, coins:utxo_common:2876] WARN Attempt N1: spendable balance 143.57607083 greater than total balance 128.57207083

28 17:03:27, utxo_common:2763] utxo_common:2733] Warning, actual block_count 1826307 less than cached tx_height 1826308 of 8d88508e0a68f59336fcd06de155b23c647c10e93f3ec47abbc83d2418a6600a
28 17:03:27, rpc:282] ERROR RPC error response: rpc:208] dispatcher_legacy:160] lp_ordermatch:4193] check_balance:28] utxo_common:2873] Internal error: Spendable balance 143.57607083 greater than total balance 128.57207083

28 17:03:37, maker_swap:755] Taker payment tx f03f0a9d69b66bab0d5df83b9d378b8b25992059d94de0f089a4c9da810c68ec
28 17:03:37, coins:rpc_clients:148] INFO Waiting for tx f03f0a9d69b66bab0d5df83b9d378b8b25992059d94de0f089a4c9da810c68ec confirmations, now 0, required 4, requires_notarization false
+--- 28 17:03:38 -------

idk what it means, but that swap also worked fine

That log might be related to this

artemii235
artemii235 approved these changes Apr 29, 2022
View reviewed changes
Copy link
Member

@artemii235 artemii235 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔥 Thanks for the fast fix!

@artemii235 artemii235 merged commit fb6890c into dev Apr 29, 2022
@artemii235 artemii235 deleted the fix-spv-proof-validation branch April 29, 2022 06:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cipig cipig cipig approved these changes

@artemii235 artemii235 artemii235 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@onur-ozkan @artemii235 @cipig