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.

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

Support posv #1815

Merged
merged 8 commits into from
May 15, 2023
Merged

Support posv #1815

merged 8 commits into from
May 15, 2023

Conversation

reddink
Copy link

@reddink reddink commented May 8, 2023

This PR enables PoSV type transactions from the likes of Reddcoin, Potcoin, et al.

The differentiator over the existing POS implementation is that the n_time is appended to the end of transaction (after locktime) rather than being at the 2nd position in the structure
Additionally, the n_time field is not used in the sign process, but is appended to the transaction after inputs are signed

@shamardy shamardy self-requested a review May 8, 2023 13:59
@shamardy
Copy link
Collaborator

shamardy commented May 8, 2023

Thanks a lot for the PR! Will try to take a look at it and review it this week :)
Just to be sure @reddink, is this supposed to fix this issue #1533?

@reddink
Copy link
Author

reddink commented May 8, 2023

Yes, it should address that issue.

Copy link
Collaborator

@shamardy shamardy left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the PR! I have two change requests :)

mm2src/mm2_bitcoin/chain/src/transaction.rs Outdated Show resolved Hide resolved
mm2src/mm2_bitcoin/script/src/sign.rs Outdated Show resolved Hide resolved
@shamardy shamardy requested a review from cipig May 9, 2023 22:29
@shamardy shamardy linked an issue May 9, 2023 that may be closed by this pull request
@cipig
Copy link
Member

cipig commented May 9, 2023

used the following settings for POT:

  {
    "coin": "POT",
    "name": "potcoin",
    "fname": "PotCoin",
    "sign_message_prefix": "Potcoin Signed Message:\n",
    "isPoSV": 1,
    "rpcport": 42000,
    "pubtype": 55,
    "p2shtype": 5,
    "wiftype": 183,
    "txfee": 100000,
    "dust": 100000,
    "mm2": 1,
    "mature_confirmations": 240,
    "required_confirmations": 5,
    "avg_blocktime": 40,
    "protocol": {
      "type": "UTXO"
    },
    "derivation_path": "m/44'/81'"
  },

and tried to withdraw some POT to some other address
get this error from chain

{"error":"rpc:211] dispatcher_legacy:141] lp_coins:3238] utxo_common:2308] rpc_clients:2186] JsonRpcError { client_info: \"coin: POT\", request: JsonRpcRequest { jsonrpc: \"2.0\", id: \"5\", method: \"blockchain.transaction.broadcast\", params: [String(\"0100000001394a6373aeb03641efd219ac37a1a068f9d988275fe588c9ac11ea488fb9a507000000006b48304502210097af008c7d929a6452e19bfe11ac595dac3bd9bd6a063245b3b1cc42517f93a402206dae1394c090f1c91284a42e9a91de804391f28ebbc8a8f77ca502acdbcdabf40121031bb83b58ec130e28e0a6d5d2acf2eb01b0d3f1670e021d47d31db8a858219da8ffffffff0200ca9a3b000000001976a9146aafbb75331085425727a78a940e9334f681b3e588ac0060dc9d020000001976a914c3f710deb7320b0efa6edb14e3ebeeb9155fa90d88ac0ed25a64\")] }, error: Response(62.171.189.243:50001, Object({\"code\": Number(1), \"message\": String(\"the transaction was rejected by network rules.\\n\\nTX rejected\\n[0100000001394a6373aeb03641efd219ac37a1a068f9d988275fe588c9ac11ea488fb9a507000000006b48304502210097af008c7d929a6452e19bfe11ac595dac3bd9bd6a063245b3b1cc42517f93a402206dae1394c090f1c91284a42e9a91de804391f28ebbc8a8f77ca502acdbcdabf40121031bb83b58ec130e28e0a6d5d2acf2eb01b0d3f1670e021d47d31db8a858219da8ffffffff0200ca9a3b000000001976a9146aafbb75331085425727a78a940e9334f681b3e588ac0060dc9d020000001976a914c3f710deb7320b0efa6edb14e3ebeeb9155fa90d88ac0ed25a64]\")})) }"}

the transaction was rejected by network rules.\\n\\nTX rejected

@cipig
Copy link
Member

cipig commented May 9, 2023

similar with RDD and this settings:

  {
    "coin": "RDD",
    "name": "reddcoin",
    "fname": "Reddcoin",
    "sign_message_prefix": "Reddcoin Signed Message:\n",
    "isPoSV": 1,
    "rpcport": 45443,
    "pubtype": 61,
    "p2shtype": 5,
    "wiftype": 189,
    "txfee": 100000,
    "dust": 100000,
    "mm2": 1,
    "mature_confirmations": 30,
    "required_confirmations": 5,
    "avg_blocktime": 60,
    "protocol": {
      "type": "UTXO"
    },
    "derivation_path": "m/44'/4'"
  },

{"error":"rpc:211] dispatcher_legacy:141] lp_coins:3238] utxo_common:2308] rpc_clients:2186] JsonRpcError { client_info: \"coin: RDD\", request: JsonRpcRequest { jsonrpc: \"2.0\", id: \"32\", method: \"blockchain.transaction.broadcast\", params: [String(\"0100000001f2c3b2eed91ba9a842c622659d2b877cce0c15224690933d0d4ef2f293b654a5000000006b483045022100f402508af6987a3553f91e722f9960b7f044457a8c96f7436003b7f434de0ad702205f5fef70e1c75a7aeea477cd1a4248fddff1c234c38df3ce0217238a3474019a0121031bb83b58ec130e28e0a6d5d2acf2eb01b0d3f1670e021d47d31db8a858219da8ffffffff0200ca9a3b000000001976a9146aafbb75331085425727a78a940e9334f681b3e588ac00e6b590000000001976a914c3f710deb7320b0efa6edb14e3ebeeb9155fa90d88ac1fdb5a64\")] }, error: Response(electrum03.reddcoin.com:50001, Object({\"code\": Number(1), \"message\": String(\"the transaction was rejected by network rules.\\n\\n0: \\n[0100000001f2c3b2eed91ba9a842c622659d2b877cce0c15224690933d0d4ef2f293b654a5000000006b483045022100f402508af6987a3553f91e722f9960b7f044457a8c96f7436003b7f434de0ad702205f5fef70e1c75a7aeea477cd1a4248fddff1c234c38df3ce0217238a3474019a0121031bb83b58ec130e28e0a6d5d2acf2eb01b0d3f1670e021d47d31db8a858219da8ffffffff0200ca9a3b000000001976a9146aafbb75331085425727a78a940e9334f681b3e588ac00e6b590000000001976a914c3f710deb7320b0efa6edb14e3ebeeb9155fa90d88ac1fdb5a64]\")})) }"}

the transaction was rejected by network rules.\\n\\n0: \\n

@shamardy
Copy link
Collaborator

shamardy commented May 9, 2023

I think there is a problem with the transaction serialization @reddink
https://github.com/KomodoPlatform/atomicDEX-API/blob/ebdc8c214c2e4b5d5a6f02b356b679a1130199e8/mm2src/mm2_bitcoin/chain/src/transaction.rs#L361-L363
n_time should only be serialized here if !self.posv

Edit: nevermind, this is what is actually done, I was using the wrong branch to check this.

@shamardy
Copy link
Collaborator

@cipig @reddink I think I found the problem, it's related to n_time as I suspected. It's never set for posv here.
https://github.com/KomodoPlatform/atomicDEX-API/blob/e6a201f1fe27366fe6a4022f3984b121e0870a91/mm2src/coins/utxo.rs#L780-L784

@reddink
Copy link
Author

reddink commented May 10, 2023

Thanks for reviewing @cipig @shamardy
ACK the suggested changes, rolling them in now

You discovered correctly the missing n_time for POSV not being set.
I am now able to create, sign and broadcast on the Reddcoin network

Do you know of some POT electrum servers i could connect to?

@reddink
Copy link
Author

reddink commented May 10, 2023

Additionally
Reddcoin configuration

  {
    "coin": "RDD",
    "name": "reddcoin",
    "fname": "Reddcoin",
    "isPoS": 0,
    "isPoSV": 1,
    "rpcport": 45443,
    "pubtype": 61,
    "p2shtype": 5,
    "wiftype": 189,
    "decimals": 8,
    "txversion": 2,
    "txfee": 0,
    "dust": 10000,
    "segwit": false,
    "bech32_hrp": "rdd",
    "mm2": 1,
    "required_confirmations": 1,
    "avg_blocktime": 60,
    "protocol": {
      "type": "UTXO"
    },
    "derivation_path": "m/44'/4'",
    "trezor_coin": "Reddcoin",
    "links": {
      "github": "https://github.com/reddcoin-project/reddcoin",
      "homepage": "https://reddcoin.com"
    }
  }
 

The tx version was also incorrect.. i suspect the same with potcoin (ver 4???)

@cipig
Copy link
Member

cipig commented May 10, 2023

Thanks for the fixes, i will try it out. Not sure if it needs txversion set, i guess not... will try that out too :-)

Do you know of some POT electrum servers i could connect to?

i use 62.171.189.243:50001

@reddink
Copy link
Author

reddink commented May 10, 2023

I think txversion is required.
Without it, tx ver is set to 1 in the actual transaction
By setting the value, the defined ver is set in the tx

@cipig
Copy link
Member

cipig commented May 10, 2023

ok, will try with txversion set
what about the txfees? couldn't find it in code, only this: src/wallet.h:static const CAmount DEFAULT_TRANSACTION_FEE = 0;
the electrum server returns a pretty high fee:

(echo '{ "id": 1, "method": "blockchain.estimatefee", "params": ["1"] }'; sleep 2) | ncat electrum02.reddcoin.com 50001

{"jsonrpc": "2.0", "result": 0.43668122, "id": 1}

what is the usual fee (in sats per kbyte)?

EDIT: could not find bech32_hrp in code either, guess we can skip that setting

EDIT2: to me it looks like the usual fee should be 100000 sats per kbyte, found things like this in code:

src/main.h:static const int64_t DUST_THRESHOLD_FEE = 100000; // 0.001 RDD
contrib/spendfrom/spendfrom.py:BASE_FEE=Decimal("0.001")

@reddink
Copy link
Author

reddink commented May 10, 2023

For this testing 100000 sats should be fine, this has been used on most electrumx servers
In my testing tonight, I just set a fixed fee amount.
Not sure why electrum is reporting a higher estimate, will take a look at that

@reddink
Copy link
Author

reddink commented May 10, 2023

Bech32 hasn't been enabled on mainnet yet, currently doing testing on testnet however, we can leave it out for the moment

@cipig
Copy link
Member

cipig commented May 10, 2023

looks good, i could send a RDD tx: https://live.reddcoin.com/tx/8006e3425a3bbac3c35f28aca4d67c7622b76cf21567221de08868c3328b7963
and a POT tx: https://chainz.cryptoid.info/pot/tx.dws?663157a74cbd7274721a4c4dd47f159051844961be59dab6a7dc1bbcc127b892 (needs a new block to show up on this explorer)

what about BIP65: https://github.com/bitcoin/bips/blob/master/bip-0065.mediawiki ?
do you plan to add/activate it on RDD? would be needed for swaps to work too.

cipig
cipig previously approved these changes May 10, 2023
@reddink
Copy link
Author

reddink commented May 10, 2023

Bip65 is being activated on mainnet with our v4.22 core wallet.
I do need to update the electrum servers to point to the new core wallets. Will get to that tomorrow.

Copy link
Collaborator

@shamardy shamardy left a comment

Choose a reason for hiding this comment

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

Only 2 more minor changes! Please also resolve the merge conflicts!

mm2src/coins/utxo.rs Outdated Show resolved Hide resolved
mm2src/mm2_bitcoin/chain/src/transaction.rs Outdated Show resolved Hide resolved
@shamardy
Copy link
Collaborator

Bip65 is being activated on mainnet with our v4.22 core wallet.

@cipig should we add RDD and POT as wallet only until we are sure swaps are working?

@cipig
Copy link
Member

cipig commented May 10, 2023

Bip65 is being activated on mainnet with our v4.22 core wallet.

@cipig should we add RDD and POT as wallet only until we are sure swaps are working?

KomodoPlatform/coins#733 ;-)

@shamardy
Copy link
Collaborator

shamardy commented May 10, 2023

Bech32 hasn't been enabled on mainnet yet, currently doing testing on testnet however, we can leave it out for the moment

It's good we added support for witnesses in the code, once Bech32 is enabled, it should work by only adding RDD-segwit, POT-segwit coins to the coins repo without more changes to the code.

shamardy
shamardy previously approved these changes May 15, 2023
@shamardy shamardy merged commit cbf312d into KomodoPlatform:dev May 15, 2023
@shamardy shamardy mentioned this pull request May 15, 2023
@ca333
Copy link

ca333 commented May 24, 2023

Thanks for the contribution @reddink

@smk762
Copy link

smk762 commented Jul 25, 2023

@reddink we seem to be facing issues with sending RDD transactions in the current apps. The error is as previously mentioned in this thread - the transaction was rejected by network rules.\\n\\nTX decode failed

Can you please check and confirm / tweak the values in https://github.com/KomodoPlatform/coins/blob/master/coins#L9333-L9360 ?

@reddink
Copy link
Author

reddink commented Jul 26, 2023

@reddink we seem to be facing issues with sending RDD transactions in the current apps. The error is as previously mentioned in this thread - the transaction was rejected by network rules.\\n\\nTX decode failed

Can you please check and confirm / tweak the values in https://github.com/KomodoPlatform/coins/blob/master/coins#L9333-L9360 ?

@smk762 do you have a copy of the raw transaction i can analyse?

@smk762
Copy link

smk762 commented Jul 26, 2023

here you go -

[
{
"error": "rpc:211] dispatcher_legacy:141] lp_coins:3365] utxo_common:2369] rpc_clients:2258] JsonRpcError { client_info: "coin: RDD", request: JsonRpcRequest { jsonrpc: "2.0", id: "12", method: "blockchain.transaction.broadcast", params: [String("0200000001532551126b30dda21ad826c440413e1db8036c3bd87bf80aa1fce91b51457de5000000006a473044022070eda67201a72e1a70416f98914acc438b9e3c09224ab733cfee7cc0034b4d6302200e946fcc1a0f00ce003c97e25dd56947a260eb4fdbbe373a6de39b3a13715674012103d8064eece4fa5c0f8dc0267f68cee9bdd527f9e88f3594a323428718c391ecc2ffffffff0200c2eb0b000000001976a914d346067e3c3c3964c395fee208594790e29ede5d88aca0e0a128010000001976a914d346067e3c3c3964c395fee208594790e29ede5d88ac3a14c164")] }, error: Response(electrum02.reddcoin.com:50002, Object({"code": Number(1), "message": String("the transaction was rejected by network rules.\n\nTX decode failed\n[0200000001532551126b30dda21ad826c440413e1db8036c3bd87bf80aa1fce91b51457de5000000006a473044022070eda67201a72e1a70416f98914acc438b9e3c09224ab733cfee7cc0034b4d6302200e946fcc1a0f00ce003c97e25dd56947a260eb4fdbbe373a6de39b3a13715674012103d8064eece4fa5c0f8dc0267f68cee9bdd527f9e88f3594a323428718c391ecc2ffffffff0200c2eb0b000000001976a914d346067e3c3c3964c395fee208594790e29ede5d88aca0e0a128010000001976a914d346067e3c3c3964c395fee208594790e29ede5d88ac3a14c164]")})) }"
}
]

0200000001532551126b30dda21ad826c440413e1db8036c3bd87bf80aa1fce91b51457de5000000006a473044022070eda67201a72e1a70416f98914acc438b9e3c09224ab733cfee7cc0034b4d6302200e946fcc1a0f00ce003c97e25dd56947a260eb4fdbbe373a6de39b3a13715674012103d8064eece4fa5c0f8dc0267f68cee9bdd527f9e88f3594a323428718c391ecc2ffffffff0200c2eb0b000000001976a914d346067e3c3c3964c395fee208594790e29ede5d88aca0e0a128010000001976a914d346067e3c3c3964c395fee208594790e29ede5d88ac3a14c164

@reddink
Copy link
Author

reddink commented Jul 26, 2023

here you go -

[ { "error": "rpc:211] dispatcher_legacy:141] lp_coins:3365] utxo_common:2369] rpc_clients:2258] JsonRpcError { client_info: "coin: RDD", request: JsonRpcRequest { jsonrpc: "2.0", id: "12", method: "blockchain.transaction.broadcast", params: [String("0200000001532551126b30dda21ad826c440413e1db8036c3bd87bf80aa1fce91b51457de5000000006a473044022070eda67201a72e1a70416f98914acc438b9e3c09224ab733cfee7cc0034b4d6302200e946fcc1a0f00ce003c97e25dd56947a260eb4fdbbe373a6de39b3a13715674012103d8064eece4fa5c0f8dc0267f68cee9bdd527f9e88f3594a323428718c391ecc2ffffffff0200c2eb0b000000001976a914d346067e3c3c3964c395fee208594790e29ede5d88aca0e0a128010000001976a914d346067e3c3c3964c395fee208594790e29ede5d88ac3a14c164")] }, error: Response(electrum02.reddcoin.com:50002, Object({"code": Number(1), "message": String("the transaction was rejected by network rules.\n\nTX decode failed\n[0200000001532551126b30dda21ad826c440413e1db8036c3bd87bf80aa1fce91b51457de5000000006a473044022070eda67201a72e1a70416f98914acc438b9e3c09224ab733cfee7cc0034b4d6302200e946fcc1a0f00ce003c97e25dd56947a260eb4fdbbe373a6de39b3a13715674012103d8064eece4fa5c0f8dc0267f68cee9bdd527f9e88f3594a323428718c391ecc2ffffffff0200c2eb0b000000001976a914d346067e3c3c3964c395fee208594790e29ede5d88aca0e0a128010000001976a914d346067e3c3c3964c395fee208594790e29ede5d88ac3a14c164]")})) }" } ]

0200000001532551126b30dda21ad826c440413e1db8036c3bd87bf80aa1fce91b51457de5000000006a473044022070eda67201a72e1a70416f98914acc438b9e3c09224ab733cfee7cc0034b4d6302200e946fcc1a0f00ce003c97e25dd56947a260eb4fdbbe373a6de39b3a13715674012103d8064eece4fa5c0f8dc0267f68cee9bdd527f9e88f3594a323428718c391ecc2ffffffff0200c2eb0b000000001976a914d346067e3c3c3964c395fee208594790e29ede5d88aca0e0a128010000001976a914d346067e3c3c3964c395fee208594790e29ede5d88ac3a14c164

Thanks for that, super helpful..

So the error returned by the electrum sever is slightly different, 'tx decode failed'.

Taking a look at the structure of that rawtx
The problem is that the tx is 4 bytes shorter than expected.
i.e it is missing a field (4 bytes) from the end of the tx.. most likely n_time.

I will take a look at the logic to see if I can track down the discrepancy.
I believe the 'coins' parameters are fine, just that something is off in komodo-defi-framework for posv

@reddink
Copy link
Author

reddink commented Jul 31, 2023

@smk762
Have a fix for the issue.

problem was at ln#780 /mm2src/coins/utxo.rs
a change there had been inadvertantly dropped
No biggie..

the line should read
let n_time = if self.conf.is_pos || self.conf.is_posv { Some(now_sec_u32()) } else { None };

have created a seperate pull request
fix for reddcoin withdrawal error #1925

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Addition of PoSV2 UTXO Coins - POT + RDD
5 participants