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

pending issue #72

Closed
MithrilMan opened this issue Jul 25, 2014 · 15 comments
Closed

pending issue #72

MithrilMan opened this issue Jul 25, 2014 · 15 comments

Comments

@MithrilMan
Copy link

ok, people reported pending transactions problem and i faced them right yesterday while i was playing

here facts:

  • i was playing, i sent a name_update to move TWO of my hunters (generals both) but one of them got killed the next block, before transaction had confirmation
  • one of my RPC call fails (i use gettransaction to see transaction status and confirmation, see ending note) saying that transaction wasn't available ("ok, no problem" I thought, that transaction isn't visible anymore because i've been killed)
  • problem is that the other hunter (the alive one) didn't get confirmation neither, standing forever in a pending status
  • from that point, i've had strange wallet behaviour (with other transaction getting pending forever)

now i've added a "Delete Transaction" in my client but that didn't sove the issue because you'll have to restart client and it's not guaranteed that the problem will be fixed

i think we have a problem here with incorrect transaction, could you check what happen when "gettransaction" try to get a transaction about a killed player with no confirmation (btw don't think the problem is there) and what happen if player is killed while he has pending moves?

don't know if the problem is only about RPC or even standard client is affected
I'll try to reproduce on testnet, anyway it seems to be an important issue to get fixed

P.S.

  • note that my client store all pending operations in an inmemory list and check every game_state if those transaction are in pending (0 confirmation) or not, then if the transaction is confirmed i trigger some client actions depending on the type of the transaction (name_new, name_firstupdate or name_update) so i have to keep that inmemory list

to check their status i call gettransaction passing the tx id of the inmemory pending operation, this cause the RPC exception when i try to ask a transaction of a death player (it's fine, talking about code flow, because then i remove it from the list, but don't know how that exception cause some problem in the wallet/daemon)
(don't tell me to use name_pending for that because i still have to know when a transaction has been confirmed because i trigger some actions, so i need an inmemory list).

@domob1812
Copy link
Contributor

I was able to reproduce this (or at least a similar) issue on test-net, based on my double-spending suspicion. I'll work on it as my next task, because I think it is important.

What do you think, would it be ok to simply delete all "stuck" transactions when a player is killed? I. e., the moves of non-killed players may get lost. Or do we have to reconstruct the name_update transactions with other inputs? The latter makes things much more complicated, especially since it may lead to different fees to be paid and things like that - so maybe better leave it to the player to re-issue the move. At least, no stuck players should already be a big improvement. Comments?

In the mean time as a work-around, I think that migrating to a completely new wallet should not be necessary - deleting all pending transactions and then re-issuing all moves of still-alive players should also work. (At least for the situations I have in mind - let me know if it is not the case.) Apart from that, having multiple outputs in the wallet to pay for transaction fees (i. e., send 10x 1 HUC to address(es) instead of 1x 10 HUC) should help to prevent the issue in general.

@MithrilMan
Copy link
Author

I don't think I've understood you well.
when you say that as a workaround, deleting all pending transactions and recreate them solve the problem, i've to say that it doesn't work, that was my first attempt (delete stuck transaction and generate a new name_update). I don't see viable workaround once you have "dirty" coins in your wallet.

to answer your question, from a player perspective having to recreate some moves would be boring and the more hunters they are controlling, the more they are going to face the problem (if they have an aggressive team i mean and join fights, the double spending issue can happen a lot of times) and from a general perspective loosing previous valid moves won't works in favour of the game.
Why do you think they would spend more fee? if the stuck one isn't accepted, doens't it mean that no fee are payd?
anyway i think i've not clear what's exactly causing the problem, woulnd't be better to just "accept" the move of a "just" (i.e. last 2 blocks) died hunter if this is causing problem?

@domob1812
Copy link
Contributor

If deleting the transactions does not solve the problem, then I have to say that I wasn't able to fully reproduce the problem. For me it worked. Can you send me a wallet (potentially on test-net if you can produce one) that shows the problem, where coins are frozen and even deleting transactions does not help?

You are right that fees are not paid for non-confirmed transactions. However, the issue is that to resolve the double-spending issue, you have to recreate the transaction "from scratch". Of course, this can be done automatically with the existing move - but it may happen that you need to include more inputs to pay the necessary fee, yielding a larger transaction, in turn yielding the necessity to pay more fees than initially. While this won't be very common (presumably), it is perfectly possible. But maybe it could be possible to recreate all moves where no larger fee is necessary (which should be almost all).

Regarding "just accepting" the move of a killed player: While it sounds easy, it would require very invasive changes to how the Bitcoin/Huntercoin transaction system works. So this is unfortunately not possible (or at least not the best solution).

@MithrilMan
Copy link
Author

the wallet i sent you times ago already have some bad coin
atm that are stuck on a name_new operation, anyway at the moment i can't reproduce because i'm on holiday (or better, i've been @ the hospital ffs... and had to move my holidays)
I'll be back around 26th, after that i could try on testnet to recreate the issue, anyway if you follow the steps above you should be able to reproduce it:
maybe you need 2 wallets to be sure, anyway:

create some player, then meet 2 of them with different color, send a destruct on one of them, then do some name_updates to others hunter and the one next to the destruct (this is important)
timing is the key, the destruct has to happen while the name_update of the hunter next to him is still in pending

if all is ok, you'll end up with the issue

@domob1812
Copy link
Contributor

What you describe is exactly what I did on test-net. But for me, while I get stuck pending operations and a "frozen" balance, things are fixed by deleting all the pending transactions. Are you sure this doesn't help?

@MithrilMan
Copy link
Author

yes i'm sure as everyone that have reported the same problem, deleting doesn't solve the problem for us. I think even that's maybe not easy to simulate exactly what's happened, using testnet that mostly process the move the next block instead of 2

moreover i don't know if the order of processing moves counts (i.e. if a player has processed its move before the destruct, maybe the tx is accepted then he die)
does name count? (are updates processed considering the name order?) and address count?
there are many variables and i've no insight to guess what's wrong, but definetly something is going wrong

@chronokings
Copy link
Owner

just a note:
We did consider this bug (originally before release). And afaik, it did auto delete transactions in which someone is in pending and is destructed by someone else.
So i'm not sure if it is the same issue or related, or if the fix was not originally applied.

@domob1812
Copy link
Contributor

In my tests, neither the actual transaction of the killed hunter nor the one depending on it (via change outputs) was deleted. Both would need to be. I think I'll try to implement that at least for now - if we are lucky, it fixes the full issue even if I'm not able to reproduce it myself. At worst it should at least be a partial solution on which to build further fixes.

So, about recreating deleted transactions of non-killed players: @MithrilMan thinks that it should definitely be implemented, so I think I'll try to do it (even if it is extra work). At least if the new transaction doesn't spend more fees. Transactions that were currency transfers will not be reconstructed - is this ok?

@chronokings
Copy link
Owner

seems ok with me if it does not open another can of worms.
Although, I'm still wondering if this is actually the issue.
I do remember something being implemented for this chain of events - possibly deleting those tx after x amount of blocks or on client restart....
Unfortunately browsing through old notes/convos around creation is virtually impossible due to communication method which was used.

@domob1812
Copy link
Contributor

Would be cool if you find some info! Otherwise, maybe I'll find something in the code. So far, it seemed as if there were no "delete" implemented (or maybe it is deleting from the mempool but not the wallet?). I think this should be done. The only "danger" in implementing this is that unconfirmed moves and transactions may be lost - which seems minor given that the chance of a reorg that revives the player is tiny, and I'll try to recover moves for non-killed players.

However, as general advice, I still stand to what I've written multiple times: Try to have more than one output in the wallet (send-to-self a few single HUCs). This should help prevent such problems (although not in every case, but at least help). I've never had any such issues, and I've tried to keep enough outputs in the wallet. (Ideally as many as you move players at the same time.)

@domob1812
Copy link
Contributor

See #74. Note that it is not a full fix, just some experiments! I found what @chronokings presumably mentioned above - EraseBadMoveTransactions. This routine does (more or less) what I wrote about above (remove invalidated moves and transactions depending on them). However, it is called only in a very limited set of situations.

With this patch, the existing RPC call "name_clean" is updated to do this full set of removals (before it did not consider game moves and was just a literal copy from Namecoin). Any chance you could try if it helps to do "name_clean" on a wallet that has the stuck transactions (with my patch)? For my own constructed situation, this fixed it. If it also fixes the "real" situation, I can extend the patch to use this mechanism more often automatically and to resurrect moves of non-killed players (as discussed above).

@chronokings
Copy link
Owner

i've committed domob's new patch.
anyone having this issue should try name_clean with the new release (will sort today or tomorrow).

Then domob can add some automation to it later on (if this is the issue).

@MithrilMan
Copy link
Author

I'd like to test it, just need to understand how
i mean, when should i call that, only when i see a pending transaction?
how to automate it? should i remember the current block when i send a transaction and then issue that name_clean after n blocks (e.g. after 10 blocks, if the tx is in pending, i should call name_clean on that name?)
what if the stuck tx is a name_new tx (it happened to me), did i have to call name_clean to the reserved name?

@domob1812
Copy link
Contributor

You don't have to automate it at all - please just try out what (if anything) a manual "name_clean" changes if you have a pending transaction / frozen coins. If it helps, I will implement automation in the client itself. This first patch is just an experiment to see whether this is enough or not.

@chronokings
Copy link
Owner

is the pending issues now fixed?

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

No branches or pull requests

3 participants