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

change address reuse and transaction time #6

Open
MithrilMan opened this issue Jan 10, 2017 · 6 comments
Open

change address reuse and transaction time #6

MithrilMan opened this issue Jan 10, 2017 · 6 comments

Comments

@MithrilMan
Copy link

In the past version of huntercoin we had by default the addressreuse setting that caused any transaction to give back the change to the same address, now is this feature with huntercore lost?

It seems that with a 20MB wallet of transaction, moves takes lot longer to be accepted (seconds) and with current block generation time so variable, there are many times a new block generate after few seconds, causing the move to go in blockchain after 3 blocks, screwing the gameplay (and i suppose this is what happened to other users that posted on the forum)

Is this because of no address reuse and so lot of unspent transactions are used? anyway the wallet is slow when he reach around 15/20 mb (non SD drive) and this would cause players to have to create new wallets very often

@domob1812
Copy link
Owner

Yes, the address-reuse "feature" is indeed not present anymore in Huntercoin Core. One reason is that I simply did not port it so far, but the other is that I don't think we should do it. Address reuse is in general not considered a good practice in the crypto community; we only added this to avoid a situation where users could lose coins because the constant using up of addresses made backups run out of cached addresses. This is no longer an issue with the new HD feature, thus I think we should not keep reusing addresses.

This has nothing do with wallet performance: What matters is the unspent outputs ("coins") in both cases, not the number of unique addresses. So from the wallet point-of-view, you always have, e. g., two coins to track; independent of whether they are on the same address or not.

If the performance is really decreased with respect to the old client, then it is a regression introduced by upstream Bitcoin since the very old code.

@MithrilMan
Copy link
Author

MithrilMan commented Jan 11, 2017

I'm concerned about game transaction, so I don't think reusing an address in this case create problems about security(anon) etc.. and could be an option to use per user case, anyway if you say this doesn't generate problem with wallet bloating ok.

I'll try to track better what's happening, anyway would sending lot of unspent coins to an address "merge them"?
I mean, if i use 10 unspent coins of 1 HUC to send them to address X, would this generate a single unspent coin (so merging the little unspent coins into one unspent coin of value 10?)
If yes, i could periodically send all coins to an address meging my little unspent.

While i'm here talking about problems, i would like to point out i found a lot of "Conflicted" transaction in my wallet, and most of them are when i try to destruct... I looked which tx were conflicting and i found they were some tx that were confirmed few blocks ahead (2 or 3), what's happening there?

Of course this often lead to lose the hunter (so 100 huc) because he missed the action i required (destruct) while the enemy didn't... this is a big problem of course, but what's the cause? there is a bug in the wallet that reuse some coins?
If you want the next time it happen i'll send you what you'd need to investigate... I read about malleability attack, could this be the case?

If you want i can open a new issue about this

P.S.
not sure if it's a coincidence, but this happens always when fighting vs ik* player, but maybe is just because he have most of the players on board

@domob1812
Copy link
Owner

No, merging of coins does not work like this. For merging, you have to explicitly create a tx with multiple inputs and sending all the value to some address of yours (it is irrelevant which addresses are involved). Are you actually sure that you have that many coins in your wallet? Try listunspent to see how many there are.

Regarding the other issue: Yes, please open a separate Github issue. Tx should be marked as conflicted only if e. g. they move a hunter that has been killed while the tx was still pending. It is not clear to me from your description whether that's actually the case or whether the tx is then confirmed later. If the former, the feature is working as intended. If the latter, then it is a bug.

@MithrilMan
Copy link
Author

MithrilMan commented Jan 11, 2017

you have to explicitly create a tx with multiple inputs and sending all the value to some address of yours (it is irrelevant which addresses are involved)

this is what i meant, sending all coins to one of my wallet address to generate 1 unspent coin and so "defragmenting" hopefully the wallet or anyway the unspent list

actually my wallet (around 25MB) has 330 unspent coins, i don't know if you consider this big or not, but comparing the time it takes my client to send a move, agianst a fresh wallet (or another wallet that have around 80 unspent coins), it's very noticeable (i'm talking about 3 seconds for each move) and it affect even the block generation time (the time my client receive the new gamestate data)

I'll try then to send my unspent coin to one address, to see if this helps and so if this points out to a scalability problem. About the other problem, I'll open a new issue with my findings and some data

@MithrilMan
Copy link
Author

MithrilMan commented Jan 22, 2017

Actually i found that a problem was related to name_list, that I used in my client: the more names you have in the wallet, the more time it takes to return.
This surprised me, how does it works, did the list got recomputed every name_list call?
Because on my tests i tried to create a wallet with lot of random names, and my stats reports that with around 100 names, it takes 5+ seconds to return (locking the wallet too if i remind right, so affecting game_status generation probably)

I was using that name_list call every new gamestate and with this fast block generation randomness, i had to remove it and hold an updated list using my game logic. But couldn't the name_list be fixed to have an inmemory rappresentation that hold the list of names? Or maybe having just the list of "alive names", because for the game purpose we are not interested in "died" names

tbh, what would be VERY useful would be to just have (and i'm pretty sure i said already this 2 years ago because faced the same "problem") to just have a "ismine:true/false" value in the player json returned in the game_getstate, isn't this trivial to do, and will help having a self-contained game status? (there, even the "isSpectator" flag should be added, because otherwise every client (mine, qt, unity, etc..) has to compute the field "stay_in_spawn_area" and it's pretty bad to not have the information right into the game status

(of course the "ismine" information is much more important.
Note that i already solved this in my client, but I think this will improve much more the huntercore itself, and even allow some other 3rd party utilities to be built on.
I'm pretty sure you understand my points about having a game state that actually holds a real game_state and not a partial rappresentation

About this issue, I think the unspent list affect times too, but i'm not yet sure, because even compacting them (sending funds to one of my wallet address, reducing the unspent list) seems to prevent that often a single move takes a couple of seconds to be issued (and so the RPC method returns after some seconds). Probably something is related to locks that apply when a new block arrive, but I'm not 100% sure, I'll try to add some timers around some of my calls to confirm if the bottleneck is the name_update call, but then I'd need your help to understand why

@MithrilMan
Copy link
Author

MithrilMan commented Jan 22, 2017

update on times: i enabled debug=1 and...

Flushed wallet.dat 18181ms
18 seconds every new block.... this mean that big wallets (mine is around 45MB and if you play a lot you'll reach that amount in a week or two, anyway even having few seconds of lock (and the time increases in a quite linear way) could make the difference between sending the transaction at the right moment or not

what can be done, except having small wallets (that isn't a solution but just a workaround?)

domob1812 pushed a commit that referenced this issue Sep 2, 2017
c521b3ac6 Merge #11: fixup define checks. Cleans up some oopses from #5.
8b1cd3753 fixup define checks. Cleans up some oopses from #5.
6b1508d6d Merge #6: Fixes typo
fceb80542 Merge #10: Clean up compile-time warnings (gcc 7.1)
0ec2a343f Clean up compile-time warnings (gcc 7.1)
d4c268a35 Merge #5: Move helper functions out of sse4.2 object
8d4eb0847 Add HasAcceleratedCRC32C to port_win.h
77cfbfd25 crc32: move helper functions out of port_posix_sse.cc
4c1e9e016 silence compiler warnings about uninitialized variables
495316485 Merge #2: Prefer std::atomic over MemoryBarrier
2953978ef Fixes typo
f134284a1 Merge #1: Merge upstream LevelDB 1.20
ba8a445fd Prefer std::atomic over MemoryBarrier

git-subtree-dir: src/leveldb
git-subtree-split: c521b3ac654cfbe009c575eacf7e5a6e189bb5bb
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

2 participants