KomodoPlatform / komodo-defi-framework

This is the official Komodo DeFi Framework repository
https://komodoplatform.com/en/docs/komodo-defi-framework/
97 stars 88 forks source link

Add support for Verus single-currency smarttx spends #2127

Closed michaeltout closed 1 month ago

michaeltout commented 1 month ago

Issue

Recently, there have been a number of reports of users not seeing funds in the Komodo Wallet and/or certain transaction outputs not being returned from the electrum listunspent API. After investigating, we've determined that this is due to Komodo Wallet lacking the ability to lookup or sign for the basic, native only smart transactions. We hope this information will enable you to make a very small change that will address the issue, and we will also take more measures in the next wallet releases to make older style transactions whenever that is a reasonable option.

Solution

It has been suggested that the issue presented above occurs server-side, and users will not be able to see funds in simple smart transaction outputs in Komodo Wallet unless the electrum servers are updated. This is not the case. Although ElectrumX is a limited scope protocol, designed for lower-function, single-currency chains, EVAL_NONE smart transaction outputs, which are sometimes used for the transactions in question, remain visible through existing ElectrumX API calls.

Allowing Komodo Wallet users to see and spend their native balance/utxos (trivial work required)

ElectrumX relies on script hashes as parameters for methods like listunspent and get_balance, thus because a simple single-currency EVAL_NONE script only has one dynamic value, the receiving address, such scripts can be generated and passed as parameters to ElectrumX methods as long as the client has access to the user's address. An example of such a script is 0400008085202f890001c07f4d0e000000001976a914aaac5e5078ff347462fa72d16ddb88a7eb50a3b288ac0000000040420f000000000000000000000000. The 20 bytes of the decoded base58 user's address are aaac5e5078ff347462fa72d16ddb88a7eb50a3b2, visible in the midsection of the hex script. The rest of the script is the framework for an EVAL_NONE smart transaction output script as shown in the linked VerusCoin codebase below. As long as these 20 bytes correspond to the user's address, calling listunspent or get_balance with a hash of this script, in addition to calling them with a hash of a P2PKH script for the user should get the total values of all their single-currency output based holdings. As long as these scripts can be constructed by Komodo Wallet, they can be found on the network. Generating signatures for these input scripts is also quite easy, and simply requires wrapping a standard signature hash in a serialised SmartTransactionSignature object, implemented in JS here in TransactionBuilder.prototype.sign.

This simple change will allow Komodo wallet to find, sign, and spend any native currency-only output funds on the Verus network, and will allow current Komodo Wallet users to spend existing funds in those outputs that they are not able to see or spend at the moment.

Extending Komodo Wallet to support multi-currency outputs and multi-currency change

Komodo Wallet will continue to be unable to spend user funds if users share keys between Komodo Wallet and Verus Wallets due to the fact that multicurrency change outputs can be generated on wallets that hold more than one currency on the Verus network.

The fundamental issue in this case is that Electrum is a database, built on top of the blockchain, indexed by transaction scripts. Electrum expects transaction scripts to be searchable and able to be used as keys by the electrum client. This design is sufficient for simple, less functional blockchains that assumes low-variability static output scripts and only one on-chain currency, where a user might only need to worry about one or two output script types and no parameters in them apart from a blockchain address. In the case of more functional smart-transaction scripts, a client would need to know exactly the currencies that were sent in an output, how much of each currency was sent, and how those currencies were ordered when serialized in order to find it on the chain if it was to be used as an index. This is not a viable, or efficient way of doing things, and thus indicates that the ElectrumX protocol is simply not sufficient for more functional blockchain protocols.

To address this in Verus Mobile (and as soon as possible, on Verus Desktop), we've created the Verus RPC protocol, as an alternative to ElectrumX link. Essentially, these RPC servers are simple subsets of the native daemon API provided by verus-cli. The server runs the daemon on the backend, and the rust middleware exposes non-wallet connected API calls for client software to interact with. In addition to providing the same functionality as a native node would for blockchain data, this gives us the added benefit of minimising client software code, as any code written to interact with the native wallet can be trivially edited to support an RPC server as well. Here is an example of a client library we use in our JS-adjacent codebases to interact with RPC servers.

In the context of the Komodo wallet, fetching UTXO/balance data from RPC servers as opposed to ElectrumX would provide all data necessary to create and spend smart transaction outputs (except for keys of course). Utxo selection and change output creation, can be done a few different ways. On Verus Mobile, currency outputs get created using this library, and put into a transaction using createUnfundedCurrencyTransfer. UTXO selection is then delegated to the RPC server through the fundrawtransaction API call, the implementation for which can be found in the fundrawtransaction function here. A list of UTXOs fetched from the getaddressutxos is received from the server, and passed into fundrawtransaction, which returns a transaction funded by utxos from the list, with the appropriate change output added. validateFundedCurrencyTransfer from the above JS library is then used to verify that the funding took place correctly, all outputs and inputs are as expected, and the transaction is safe to be signed and sent.

Smart TX References

Smart transaction scripts are comprised of serialised constructs, shown in this file in the core daemon repository or this file in a typescript implementation.

ca333 commented 1 month ago

Thank you for your detailed report. We appreciate your thorough investigation into the matter.

It appears that the issue is rooted in the limitations of the ElectrumX Verus implementation, particularly in handling Verus specific native-only smart transactions. While ElectrumX is indeed designed with a limited scope, we recognize the need for it to serve more complex use cases such as those you’ve described.

To address this, we suggest three potential pathways:

(i) Extend ElectrumX: Enhance the ElectrumX interface to support the required data retrieval and compatibility with Verus Smart TX. This would involve adapting ElectrumX to handle the script hashes and smart transaction outputs you detailed.

(ii) Extend Verus RPC Protocol Interface: Modify your existing Verus RPC protocol to align with the ElectrumX API interface 1:1. By doing so, we could seamlessly switch to using your RPC endpoints, thereby providing the necessary support for Verus smart transactions without altering our current client-side implementations.

(iii) Submit a Pull Request: Given that the required changes are described as "trivial work," we encourage you to submit a pull request with the necessary modifications. This would expedite the process and directly integrate the required functionality for the Verus ecosystem.

All approaches aim to resolve the current visibility and transaction signing issues for Verus users, ensuring a smoother user experience and broader functionality.

As this issue ticket does not pertain directly to the Komodo Wallet / Komodo DeFi Framework itself, but rather to the server-side infrastructure, we will be closing this ticket. We look forward to any developments on your end and remain open to integrating a compatible solution once available.

Thank you for your collaboration and understanding.

michaeltout commented 1 month ago

Thank you for your detailed response, perhaps you missed the second paragraph, in which the first sentence explains how this is in fact not a server-side problem, but an issue that pertains directly to the Komodo Wallet / Komodo DeFi framework. I've pasted it below to highlight just in case:

It has been suggested that the issue presented above occurs server-side, and users will not be able to see funds in simple smart transaction outputs in Komodo Wallet unless the electrum servers are updated. This is not the case. Although ElectrumX is a limited scope protocol, designed for lower-function, single-currency chains, EVAL_NONE smart transaction outputs, which are sometimes used for the transactions in question, remain visible through existing ElectrumX API calls.

The section titled Allowing Komodo Wallet users to see and spend their native balance/utxos then describes exactly the work required to the KW wallet, as requested by gcharang on the Komodo discord. There is no change to ElectrumX that can rectify this issue, nor are there any technical solutions that can be applied to Verusd-RPC either. None of the technical pathways you've suggested would address the issues described. It seems that perhaps this misunderstanding might have come from an interpretation of the original post, so I'm happy to communicate with any KW dev involved in active development to assist as well (if the provided explanation is not sufficient).

Looking forward to a response and the swift resolution of an issue that seems prevalent for a number of KW users at the moment.

Thank you for your attention.

cipig commented 1 month ago

is this related https://github.com/KomodoPlatform/komodo-defi-framework/pull/2053 ?

ca333 commented 1 month ago

Thank you for the clarification. We would like to proceed with further testing to validate the information provided and address the issue effectively.

Could you please provide the following details to assist us in this process:

(i) Address/Script Hash: Share the specific address / scripthash associated with these funds (referenced example).

(ii) ElectrumX API Response: Include the response from the blockchain.scripthash.listunspent call that contains these EVAL_NONE smart transaction outputs alongside normal UTXOs.

(iii) Steps to Reproduce: Outline the exact steps you followed to obtain these EVAL_NONE smart transaction outputs in conjunction with normal UTXOs.

This information will enable us to conduct thorough testing and verify the compatibility. We appreciate your cooperation / support and look forward to resolving this issue for your and our users.

cipig commented 1 month ago

i tested one specific address from a user who reported that he doesn't see all UTXOs in Komodo Wallet it was this address: https://insight.verus.io/address/RTRJJLED2CYAJ7CU3oQJZRizaV48ATH1kS and it had 2 UTXOs, a normal one, 84fb501a6d1d31773bee80bbd74469ded7fbb225ffd657f94eb8bc3e4e827f85 and one with OP_CHECKCRYPTOCONDITION: 788f2114d1b6a5bdd587ade6d040b37d8b145c873b76fe4cebcba41671efe154 that didn't show up in response from listunspent:

(echo '{ "id": 1, "method": "blockchain.scripthash.listunspent", "params": ["565cfedcf3053f64f64b8e470887888f613d1a6edb7f47b3527c375c053ce067"] }'; sleep 0.5) | ncat el0.veruscoin.io 17485 | json_pp
{
   "id" : 1,
   "jsonrpc" : "2.0",
   "result" : [
      {
         "height" : 3021059,
         "tx_hash" : "84fb501a6d1d31773bee80bbd74469ded7fbb225ffd657f94eb8bc3e4e827f85",
         "tx_pos" : 2,
         "value" : 123336
      }
   ]
}

the vout from 788f2114d1b6a5bdd587ade6d040b37d8b145c873b76fe4cebcba41671efe154 looks like this and is not included in blockchain.scripthash.listunspent:

   "vout" : [
      {
         "n" : 0,
         "scriptPubKey" : {
            "addresses" : [
               "RTRJJLED2CYAJ7CU3oQJZRizaV48ATH1kS"
            ],
            "asm" : "0403000000 OP_CHECKCRYPTOCONDITION 040300010114c6fac8e9169b1a9430260dfc0c558d15b3348a45 OP_DROP",
            "hex" : "050403000000cc1a040300010114c6fac8e9169b1a9430260dfc0c558d15b3348a4575",
            "reqSigs" : 1,
            "spendableoutput" : true,
            "type" : "cryptocondition",
            "unknown" : ""
         }
   ]

i converted the address to scripthash with this python script:

import os
import ecdsa
import hashlib
import base58
import binascii
import codecs
import struct

denariusAddress= "RTRJJLED2CYAJ7CU3oQJZRizaV48ATH1kS"
print("D Addy:" + denariusAddress)

#base58decode denarius address
addrToBytes = base58.b58decode(denariusAddress)
print(addrToBytes)
decodedToHex = addrToBytes.hex()
print(decodedToHex)

#remove prefix
removeZeroBytes = 2
decodedToHexnoPrefix = decodedToHex[removeZeroBytes:]
print(decodedToHexnoPrefix)

#remove checksum
removeChecksum = 40
decodedNoPrefixnoChecksum = decodedToHexnoPrefix[:removeChecksum]
print(decodedNoPrefixnoChecksum)

#Add OP_DUP OP_HASH160 OP_EQUALVERIFY OP_CHECKSIG
opDup = "76"
opHash160 = "A9"
opsBuffer = "14"
opEqualVerify = "88"
opChecksig = "AC"

preparedtoHash = opDup + opHash160 + opsBuffer + decodedNoPrefixnoChecksum + opEqualVerify + opChecksig
print(preparedtoHash.upper())

hashedKey = codecs.decode(preparedtoHash.upper(), 'hex')
s = hashlib.new('sha256', hashedKey).digest()
r = hashlib.new('ripemd160', s).digest()

convertBigEndian = (codecs.encode(s, 'hex').decode("utf-8"))
print(convertBigEndian)

scriptHash = codecs.encode(codecs.decode(convertBigEndian, 'hex')[::-1], 'hex').decode()
print(scriptHash)

which generates this output

python3 convert_address 
D Addy:RTRJJLED2CYAJ7CU3oQJZRizaV48ATH1kS
b'<\xc6\xfa\xc8\xe9\x16\x9b\x1a\x940&\r\xfc\x0cU\x8d\x15\xb34\x8aE\x90@\xaf\xd7'
3cc6fac8e9169b1a9430260dfc0c558d15b3348a459040afd7
c6fac8e9169b1a9430260dfc0c558d15b3348a459040afd7
c6fac8e9169b1a9430260dfc0c558d15b3348a45
76A914C6FAC8E9169B1A9430260DFC0C558D15B3348A4588AC
67e03c055c377c52b3477fdb6e1a3d618f888708478e4bf6643f05f3dcfe5c56
565cfedcf3053f64f64b8e470887888f613d1a6edb7f47b3527c375c053ce067

used 565cfedcf3053f64f64b8e470887888f613d1a6edb7f47b3527c375c053ce067 in blockchain.scripthash.listunspent, which showed only one of the 2 UTXOs

cipig commented 1 month ago

what about this: https://github.com/spesmilo/electrumx/blob/master/electrumx/lib/script.py#L99-L106 ?

class ScriptPubKey:
    '''A class for handling a tx output script that gives conditions
    necessary for spending.
    '''

    TO_ADDRESS_OPS = (OpCodes.OP_DUP, OpCodes.OP_HASH160, -1,
                      OpCodes.OP_EQUALVERIFY, OpCodes.OP_CHECKSIG)
    TO_P2SH_OPS = (OpCodes.OP_HASH160, -1, OpCodes.OP_EQUAL)
    TO_PUBKEY_OPS = (-1, OpCodes.OP_CHECKSIG)

not an expert, but to me it looks like this needs to be changed to include OP_CHECKCRYPTOCONDITION

michaeltout commented 1 month ago

Thank you for the detailed responses, I'll try my best to address them, and hopefully we can come to a solution for KW users.

Regarding your first response, with the three requests @ca333, I want to note that I'm happy to help for the benefit of Komodo users, but really this problem does not exist within Verus wallets, this is a KW specific issue. The assistance I provide is voluntary, to help those experiencing problems with the Komodo wallet, for the good of the crypto ecosystem in general.

AFAIK Komodo is a company structure, with salaried developers who work full time on maintaining the Komodo Wallet, so I would assume a problem relating to how KW interprets txs on a network that multiple KW coins run on, used by a large number of KW users, would be something that the KW development team is interested in fixing.

All this being said, to address your points, I've put together a very simple boilerplate that illustrates how to communicate with ElectrumX to retrieve unspent tx outputs and balances from smart transaction single-currency outputs, here.

@cipig I also appreciate your detailed response as it has led me to realise that the script I included in the initial issue was a typo, and was accidentally copied from a different script, a single-currency smarttx script looks like 050403000000cc1a040300010114c6fac8e9169b1a9430260dfc0c558d15b3348a4575, as you can see in your vout from 788f2114d1b6a5bdd587ade6d040b37d8b145c873b76fe4cebcba41671efe154. As you can also see in the code I've provided to assist here, all you need to do to recognise balances on these outputs is use the correct script hash, by splicing the 20 bytes of a base58 address into a basic smarttx script, and then using the hash of that for blockchain_scripthash_listunspent and blockchain_scripthash_getBalance. You then add the unspent transaction outputs and balances you get from those results, to the unspent transaction outputs you would get from using those APIs on a P2PKH script, and you will get a complete view of all balances/utxos on an address contained in single-currency outputs.

As shown in the code I've written to illustrate all this, it is trivial. To sign these scripts I'd reference what I wrote in the original post:

Generating signatures for these input scripts is also quite easy, and simply requires wrapping a standard signature hash in a serialised SmartTransactionSignature object, implemented in JS here in TransactionBuilder.prototype.sign.

If you would like more information on how EVAL_NONE smart transaction scripts are generated (regardless of the fact that you can splice in an address to a pre-built script), I would recommend looking at line 63 here, under test (de)serialize a basic output script.

I hope this effectively illustrates the fact that the ElectrumX server does not require any changes to interpret these types out outputs and balances. Let me know if anything is still unclear. I will be quite busy this week due to Consensus 2024, but this should provide everything needed to implement the relevant changes.

cipig commented 1 month ago

I just installed Verus Mobile in an attempt to reproduce the situation of the users. I sent 1 VRSC from Mobile and the TX is showing up just fine in listunspent of the electrum:

(echo '{ "id": 1, "method": "blockchain.scripthash.listunspent", "params": ["225b1c0df4720e3a80d7515a22dd0e1d7072502739f179867656f7411b1285cb"] }'; sleep 0.5) | ncat el0.veruscoin.io 17485 | json_pp
{
   "id" : 1,
   "jsonrpc" : "2.0",
   "result" : [
      {
         "height" : 3054223,
         "tx_hash" : "327e5dfaf9a364d9b3741a317ecc1b146c2a9023dcb7e14d388d39405a37a32c",
         "tx_pos" : 0,
         "value" : 100000000
      },
      {
         "height" : 3056367,
         "tx_hash" : "e3679f5a1d27c56475e02189c8327ec6894037ef0ee98c51711ab1951e211dae",
         "tx_pos" : 0,
         "value" : 1800000000
      },
      {
         "height" : 3066766,
         "tx_hash" : "3fe8b260681ab7a0fe46947ae22d8c4ac0df55fffbd585966b68f1cc2671827d",
         "tx_pos" : 0,
         "value" : 99990000
      }
   ]
}

https://insight.verus.io/tx/3fe8b260681ab7a0fe46947ae22d8c4ac0df55fffbd585966b68f1cc2671827d shows up in listunspent... so i wonder what the user did... or was this changed in the meantime in Verus Mobile code?

EDIT Found it in the changelog: Fix issue where P2PKH outputs weren't being created when able (instead of smarttx single-currency outputs) from yesterday. @michaeltout how can i create a smart-tx with a vout to RMPFzUymdaxwsHNzJvsA8sd7UZDLkDRQhD for testing purposes?

michaeltout commented 3 weeks ago

Hi, just recently returned from Consensus. You are correct, the newest version of the Android wallet will create P2PKH outputs in certain cases where it used to create smarttx outputs. The latest version on the Apple App store however still creates these smarttx outputs, so for testing I would recommend trying with that one. There are ways involving the manual creation of txs as well, but if its possible for you to test on iOS, it would be the easiest solution.