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

Submit all results instead of trying to guess when they're stale. #3

Closed
wants to merge 1 commit into from

Conversation

forrestv
Copy link

@forrestv forrestv commented Feb 8, 2012

Simply forgetting about solutions after a long poll comes in can cause pools to
lose block solutions and miners to lose shares. Not all long polls mean that
all past work is useless - for example, merged mining pools might trigger a
long poll on all of the different chains, but a miner's results might be a
block solution to one of the other unchanged chains.

Simply forgetting about solutions after a long poll comes in can cause pools to
lose block solutions and miners to lose shares. Not all long polls mean that
all past work is useless - for example, merged mining pools might trigger a
long poll on all of the different chains, but a miner's results might be a
block solution to one of the other unchanged chains.
@CFSworks
Copy link
Collaborator

CFSworks commented Feb 9, 2012

It's better to focus on improving stale-work identification rather than remove it entirely. While potentially throwing out a good result or two is bad, wasting network bandwidth to send in stale shares isn't much better. (And this also drives up the rejected shares number, making users unhappy.)

It seems the problem is not that a long-poll causes work to be tossed (Phoenix doesn't assume this anyway - pushed work and polled work are treated the same), but that a change in identifier signals the miner to immediately regard all shares based on the old work to be stale. Perhaps it would be better to rely on the previous block hash field for stale identification instead? I can think of no situation where submitting results based on an outdated previous-block-hash field is beneficial.

@runeksvendsen
Copy link

I can think of no situation where submitting results based on an outdated previous-block-hash field is beneficial.

In the case of p2pool, where the miner is working on a separate chain with lower difficulty, throwing away work done after a long-poll might cause us to lose a valid Bitcoin block. If the difficulty target of the Bitcoin block chain is met for this share, a valid Bitcoin block is thrown away, while if only the difficulty target of the p2pool share chain is met, a stale share is correctly discarded.

@CFSworks
Copy link
Collaborator

CFSworks commented Feb 9, 2012

Forgive me if this is ignorant since I have only a cursory understanding of P2Pool, but...

Phoenix doesn't throw out shares after a long-poll. It only throws out shares if it deems that the newly-found result is based on a block which is out of date. Currently, this decision is made by comparing the work identifier, which forrestv pointed out is a mistake (since changes in the work identifier do not necessarily indicate that the previous getwork is stale)

Also, if a Bitcoin block's "previous block" field is not the latest, then the new block would be a fork. Therefore, throwing out a stale block, even though its hash meets the full network target, is correct behavior, since the Bitcoin network itself wouldn't accept it because the block is not built on top of the highest block.

The changes in this pull request would certainly work fine for P2Pool, where the local P2Pool node can sift through the good/bad shares itself. However, when used with traditional pools, these changes will cause Phoenix to submit more bad shares to the pool server, which wastes bandwidth, upsets users (they don't like seeing high reject rates), and in cases where pools reward users for low stale rates, forfeits those rewards. So I still feel that it would be best to improve Phoenix's stale result detection rather than remove it completely.

@forrestv
Copy link
Author

forrestv commented Feb 9, 2012

I can think of no situation where submitting results based on an outdated previous-block-hash field is beneficial.

What if you're mining on a merged mining pool, receive a long poll because a Bitcoin block was found, but then you find a Namecoin block in a previous work unit?

@CFSworks
Copy link
Collaborator

That makes a lot more sense.

Now the solution is a lot more clear: instead of submitting all results blindly, the stale work identification code should instead be updated to test if any of the chains is unchanged, meaning that the server would still accept results.

@forrestv
Copy link
Author

There is no way to tell if any of the chains are unchanged, because they're all hidden within the merkle root of a work unit.

The only solution that I can see is submitting everything. I don't think there would be much of a negative impact at all - the amount of data transferred while submitting a solution is tiny, smaller than a getwork. Masking stales by forgetting about them does not solve any problems - It simply creates new problems for pools by discarding potentially useful work.

In addition, DiabloMiner had always submitted all shares.

@CFSworks
Copy link
Collaborator

Ah, now I understand the problem a lot better, thank you... But just a couple more things:

There is no way to tell if any of the chains are unchanged, because they're all hidden within the merkle root of a work unit.

Isn't this the purpose of X-Work-Identifier? If not, maybe getwork should be modified so the pool can indicate the status of every block chain the server is working on... :)

Masking stales by forgetting about them does not solve any problems

I recall a pool (I forget which) that gave a share bonus to miners who had very low stale rates, which is one of the motivations for being so careful with stales in the first place. In the case of non-merged pools (which was, at the time this code was written, every pool), dropping shares determined to be stale keeps the accepted ratio up and makes users and pool operators happy.

The problem is pools that want to know about old shares anyway. Is there a header for this? I was looking at "submitold" but that is only sent with longpoll responses (and so, can't be used to tell Phoenix that stale shares are OK until a longpoll comes back).

So, that's three good alternative solutions:

  • Preferably, the pool server indicates the status of each chain (in the X-Work-Identifier or a new header).
  • The pool server indicates that it wants the stale work filter to be disabled.
  • User-specified config parameter to enable submitting all shares.

However, if we simply use this new behavior unconditionally, users will likely just complain that their accept ratio is worse and will probably want to downgrade - which doesn't help anybody. :(

Again, thanks for taking the time to explain this - please let me know what you think.

@forrestv
Copy link
Author

Of those three, I think that the second is best - implementing support for "submitold". (submitold only being sent with longpolls won't be a problem, because Phoenix only discards stales after its received a longpoll, at least usually.)

The first is too complex and the last is too prone to users not caring. After all, this won't help users much because the shares are already stale..

@CFSworks CFSworks closed this Feb 17, 2012
@CFSworks
Copy link
Collaborator

I have closed this since it's been implemented in Phoenix 2.

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.

None yet

3 participants