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

Ballots may be assigned an incorrect proof #6607

Closed
jhoneycutt opened this issue Oct 25, 2019 · 2 comments · Fixed by brave/brave-core#3820
Closed

Ballots may be assigned an incorrect proof #6607

jhoneycutt opened this issue Oct 25, 2019 · 2 comments · Fixed by brave/brave-core#3820

Comments

@jhoneycutt
Copy link

jhoneycutt commented Oct 25, 2019

Description

After retrieving a set of ballot proofs, PhaseTwo::ProofBatchCallback may assign the incorrect proof to a ballot.

When iterating the set of ballots, the function checks that the surveyorId matches, but this is insufficient to identify a matching ballot, as ballots for different viewing IDs may share a surveyorId.

I believe, but have not verified, that these votes are delivered to the server and appear to it as duplicate votes. Because of #6775, duplicate votes rejected by the server are treated by the client as successful and removed from the outgoing queue. Thus they are dropped by both the client and the server and effectively "lost."

Found while investigating #6545.

Steps to Reproduce

Steps copied from #6545. Note that it does not always reproduce.

  1. enable rewards with short contribution interval on staging. Launched with --enable-logging=stderr --vmodule=rewards=6 --rewards=staging=true,reconcile-interval=10,short-retries=true to see log messages, use staging env, set short contribution interval, and short retry interval.
  2. Recovered wallet with 30 BAT
  3. set AC amount to Up to 20 BAT
  4. add verified and non verified publishers to AC table
  5. add verified recurring tips in this order (5, 10, 10, 10, 1)
  6. wait for monthly contribution to complete

Actual result:

Ballots may be assigned an incorrect proof.

Reproduces how often:

My sample size is small, but I see it occur perhaps 1/3 times with the above steps. The server-side folks may be able to comment on how often they see duplicate votes.

@srirambv
Copy link
Contributor

Verification passed on

Brave 0.71.114 Chromium: 78.0.3904.97 (Official Build) (64-bit)
Revision 021b9028c246d820be17a10e5b393ee90f41375e-refs/branch-heads/3904@{#859}
OS Windows 10 OS Version 1903 (Build 18362.418)
  • Verified steps from issue description
  • Verified log doesn't capture Invalid surveyor proof 6607.log
  • Ran through the test 2 times

@LaurenWags
Copy link
Member

LaurenWags commented Nov 11, 2019

Verified passed with

Brave 0.71.114 Chromium: 78.0.3904.97 (Official Build) (64-bit)
Revision 021b9028c246d820be17a10e5b393ee90f41375e-refs/branch-heads/3904@{#859}
OS macOS Version 10.13.6 (Build 17G5019)

Verification passed on

Brave 0.71.114 Chromium: 78.0.3904.97 (Official Build) (64-bit)
Revision 021b9028c246d820be17a10e5b393ee90f41375e-refs/branch-heads/3904@{#859}
OS Ubuntu 18.04 LTS
  • Verified steps from issue description
  • Verified log doesn't capture Invalid surveyor proof

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