Fix ExtractDestination() for ColdStaking

navcoin / navcoin-core

bitcoin-core 0.13 fork ported for NavCoin

MIT License

123 stars 92 forks source link

Fix ExtractDestination() for ColdStaking #898

Closed aguycalled closed 2 years ago

aguycalled commented 2 years ago

What to test:

Cold Staking works correctly with this patch

mxaddict commented 2 years ago

Building with Ubuntu 20.04

mxaddict commented 2 years ago

Testing the cold staking now:

Will let this run for a while, will approve once I have had the cold staking running for about an hours time

aguycalled commented 2 years ago

Good! It should also get tested that spendable and stakable balances are correctly shown, my tests were successful on that regard.

mxaddict commented 2 years ago

Good! It should also get tested that spendable and stakable balances are correctly shown, my tests were successful on that regard.

So far tha balances are correct on the UI as well, but will monitor it as it keeps staking.

mxaddict commented 2 years ago

@aguycalled "Pending" balance is only for when a TX is in the stem/mempool but is not yet staked in a block correct?

aguycalled commented 2 years ago

yes

mxaddict commented 2 years ago

I've setup 2 cold staking nodes and 1 spending node, seems to be working nicely so far, will let these run for a while longer:

chasingkirkjufell commented 2 years ago

building with ubuntu 20.04 as well, will report any findings

chasingkirkjufell commented 2 years ago

when i try to send coins to the cold staking address the balance isn't correct. steps: create 2 wallets, create a coldstaking address v1 with wallet 1 benign spending and wallet 2 being staking disable staking for both wallets to have better control send multiple transaction from wallet 1 to the cold staking address generate a block

chasingkirkjufell commented 2 years ago

so it looks like the staking wallet is showing the correct balance since actually there are only 2 groupings that went through? history shows all 5 though. window on the left is from spending wallet while on the right is the staking wallet

mxaddict commented 2 years ago

@chasingkirkjufell I did not run into any balance issues with my testing, can you give me steps to replicate?

aguycalled commented 2 years ago

I haven't been able to replicate it. Most likely case is that the first transactions were spent in the following (in case they were sent to a cold staking address with a spending address of that node). Will wait to hear more details to see if we can replicate/confirm.

chasingkirkjufell commented 2 years ago

I haven't been able to replicate it. Most likely case is that the first transactions were spent in the following (in case they were sent to a cold staking address with a spending address of that node). Will wait to hear more details to see if we can replicate/confirm.

I noticed that this issue is not related to this PR so I am going to approve this PR for now. Lke your said, it may be because of the outputs were spent again but the transaction history is confusing. For what it's worth, here are a bit more detailed steps.

creating 2 wallets and disable staking
connect 2nd wallet to first
generate 300 blocks on 1st wallet
create a coldstaking address v1 with wallet 1 spending and wallet 2 staking
send 5 txs to coldstaking address
generate more blocks
the tab will show all 5 txes but only 2 are actually valid. repair wallet or zapwallettxes does not fix it