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

expose 'split' and 'merge' in GUI #50

Closed
dooglus opened this issue Jul 26, 2014 · 7 comments
Closed

expose 'split' and 'merge' in GUI #50

dooglus opened this issue Jul 26, 2014 · 7 comments

Comments

@dooglus
Copy link
Collaborator

dooglus commented Jul 26, 2014

The client will currently split an output if it stakes within 28 hours of its most recent transaction (previous stake, or previous movement) regardless of the size of the output. It could be a tiny output that just happened to get lucky. I would argue that such an output doesn't need splitting.

It will also re-merge those split outputs the next time either of them stakes if they are both less than 1000 CLAMs.

This results in split - merge - split - merge cycles.

The re-merging is particularly annoying, since it destroys the age of two outputs at once, while only providing a single reward.

We should:

  1. make the split and merge happen based on output size and current difficulty, rather than the random luck of a particular output

  2. expose the thresholds for split and merge (however the end up after 1) above) in the GUI and RPC interface

  3. set reasonable defaults - probably with 'merge' disabled by default

@ClamProject
Copy link
Collaborator

As per our previous conversation, I 100% agree.

If you can get together a method that accurately shoots for efficiency based on output size and difficulty; I am all for such a change.

You seem to have the best grasp on the 'coinDays' v. diff. math.

I think that exposing it to the GUI such that users can deviate from that "reasonable default" if they choose is also a great idea.

A "no brainer", as they say.

@l0rdicon
Copy link
Collaborator

Even without the lottery this seems like a worthwhile improvement.

I'll also see about bringing in the novacoin rpc functions for splitting up inputs. It will give a simple console based option as well

@l0rdicon l0rdicon added this to the 1.4.4 milestone Dec 26, 2014
@tryphe tryphe self-assigned this Dec 27, 2014
@tryphe
Copy link
Collaborator

tryphe commented Dec 27, 2014

I was going to put this in a release before but scrapped the code at the last second. I have a nice algorithm to split the piles evenly while appending change to the last output. We can set a max "split" tx amount if we want as well. I think having this as a checkbox option (off by default) would be wise, like coin control.
Tell me what you think.

Here's the splitting portion of the code in wallet.cpp, It's tested:
http://codepaste.net/g6twqf

@l0rdicon
Copy link
Collaborator

It looks good to me, although you should use nSplitSize instead of a local SPLIT_AMOUNT as dooglus added in a way to set the size to spilt in the conf / passing it as an option.

@l0rdicon
Copy link
Collaborator

Actually, after looking again it looks like dooglus already handled the algo as well

https://github.com/nochowderforyou/clams/blob/1.4.3.2/src/wallet.cpp#L2077-L2097

All that need to be done it looks like is extending it to QT

oh, and completely the merge part of it.

@l0rdicon
Copy link
Collaborator

for that it seem all that needs to be done is exposing

GetStakeCombineThreshold()

https://github.com/nochowderforyou/clams/blob/1.4.3.2/src/wallet.cpp#L35

@creativecuriosity
Copy link
Collaborator

Merging the GUI exposure part of this issue into #208.

Closing.

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

No branches or pull requests

5 participants