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

Calls to BAT-Native-Ledger are blocked when a reconciliation is in progress #3393

Closed
jasonrsadler opened this issue Feb 16, 2019 · 17 comments · Fixed by brave/brave-core#1800

Comments

@jasonrsadler
Copy link

jasonrsadler commented Feb 16, 2019

Test plan

See brave/brave-core#1800

Description

When a reconciliation is in progress, calls to bat-native-ledger are blocked and queued

Steps to Reproduce

  1. Open Brave, enable Rewards, and fund wallet
  2. Visit a verified publisher and have added to auto-contribute list
  3. Trigger an auto-contribution reconciliation
  4. Visit any site and open rewards panel

Additional:
During contribution process, go to Rewards, auto-contribute settings and change some settings. Quit the browser (you may have to 'Force Quit'/'End Task') and open publisher_state. None of the changes will have been recorded.

During contribution, sites won't add to autocontribute list until after contribution is completed.

Actual result:

Only wallet summary is displayed because calls to get publisher related information are blocked.
Leave panel open until reconciliation is complete and panel will work properly.

Expected result:

Publisher information should show in the panel

Steps to Reproduce

  1. Open Brave, enable Rewards, and fund wallet
  2. Visit a verified publisher and have added to auto-contribute list
  3. Trigger an auto-contribution reconciliation
  4. Go to Rewards and open backup wallet modal

Actual result:

Backup words do not appear.

Expected result:

Backup words should appear.

Reproduces how often:

easily

Brave version (brave://version info)

Brave 0.59.35 Chromium: 72.0.3626.81 (Official Build) (64-bit)
Revision ac8b982e05014492d1bd7d317628a4f22a97ffa0-refs/branch-heads/3626@{#796}
OS Mac OS X

Reproducible on current release:

  • Does it reproduce on brave-browser dev/beta builds?
    yes

Website problems only:

  • Does the issue resolve itself when disabling Brave Shields?
  • Is the issue reproducible on the latest version of Chrome?

Additional Information

cc @NejcZdovc @bridiver @ryanml The STRs above are just a couple examples. It seems any call going into bat-native-ledger during reconciliation is queued up until after reconciliation process (right before voting). Seems to have a lesser impact if a non-autocontribute reconciliation is in progress but still somewhat noticeable.

Possibly related: #3345, #3039

@jasonrsadler jasonrsadler added this to Untriaged Backlog in Rewards via automation Feb 16, 2019
@GeetaSarvadnya
Copy link

Unable to reproduce the issue on Windows 10 x64 - 0.59.35 and 0.60.34,
But onething observed is, if we keep open the BR panel for any site, after reconcilation the wallet balance did not go down immediately in BR Panel in 0.60.34 need to refresh the page once or twice (could be because of #3345).
On 0.59.35 works fine, after reconcilation wallet balance goes down immediately in BR panel for any site.

@jasonrsadler
Copy link
Author

@GeetaSarvadnya Are you able to reproduce on nightly (master)? I just reproduced on latest master on Win 10 Pro x64.

@GeetaSarvadnya
Copy link

@jasonrsadler No, I haven't tried it on master, will try later.

@LaurenWags
Copy link
Member

LaurenWags commented Feb 18, 2019

@jasonrsadler reproduced panel not showing publisher info during contribution on macOS

Brave 0.60.28 Chromium: 72.0.3626.96 (Official Build) beta(64-bit)
Revision 84098ee7ef8622a9defc2ef043cd8930b617b10e-refs/branch-heads/3626@{#836}
OS Mac OS X

@jasonrsadler
Copy link
Author

jasonrsadler commented Feb 18, 2019

Repro'd on 59.35 macOS. Added additional info above first STR. Oddly, on release, the only thing that works from native-ledger is getting the recovery words but not sure if that was pre-cached. @LaurenWags can you repro on 59.35?

Also repro'd on Ubuntu 18.04

@LaurenWags
Copy link
Member

@jasonrsadler reproduced the panel showing summary info and unable to add sites to table during contribution with 0.59.35. Did not reproduce the backup words or settings issues.

Brave 0.59.35 Chromium: 72.0.3626.81 (Official Build) (64-bit)
Revision ac8b982e05014492d1bd7d317628a4f22a97ffa0-refs/branch-heads/3626@{#796}
OS Mac OS X

@kjozwiak
Copy link
Member

@jasonrsadler this has been an issue for a while now. Sometimes when you make a tip and it takes a bit longer for it to get through the process, you'll run into issues with the panel as @LaurenWags described below. Some of the symptoms of this happening are the following:

  • your balance appears as 0 even though you have BAT until the tip is sent server side
  • the rewards panel doesn't display the publishers information until the tip is sent server side
  • the rewards panel shows that a publisher isn't verified (even though they are) until the tip is sent server side

Quick example of the issue occurring under 0.60.44 Chromium: 72.0.3626.109 which is an RC:

rewardsexample

Assuming this is the same issue that you're seeing when the a-c goes through reconcilement.

@jasonrsadler
Copy link
Author

jasonrsadler commented Feb 21, 2019

The problem isn't as obvious under tipping as it is under auto-contribute (however, if I run it in dev, I can see it blocking on some things momentarily) as auto-contribute has the entire batch proofing system for ballots to run through. It is during the batch proofing process that everything gets blocked. Tipping is a direct send to one publisher (with no ballots, voting, etc) so I don't think you'll see this as noticeably as when an a-c contribution is going through. // bad_info 😊

@NejcZdovc
Copy link
Contributor

@jasonrsadler that is not correct. We do everything the same for ac as we do for tipping. So for tipping we do ballots and voting as well

@jasonrsadler
Copy link
Author

Good to know. I'll double test under that as well.

@NejcZdovc NejcZdovc added priority/P5 Not scheduled. Don't anticipate work on this any time soon. dev-concern labels Feb 26, 2019
@NejcZdovc NejcZdovc moved this from Untriaged Backlog to P3, P4, & P5 Backlog in Rewards Feb 26, 2019
@kjozwiak
Copy link
Member

@NejcZdovc this is a bigger/more noticeable issue with the 0.61.x. While the contribution is occurring after a tip, clicking on the rewards icon will display the following:

screen shot 2019-02-26 at 5 04 12 pm

Once the contribution goes through, the banner will load correctly. However, at first, I thought that the banner ended up crashing as it it took ~1min for the tip to go through. Example of the issue occurring:

rewardsbannerissue

Used the following build to reproduce the above:

Brave 0.61.38 Chromium: 73.0.3683.39 (Official Build) beta(64-bit)
Revision cc53b0e12fcaf42e4bab8d6c23bd4fb7aae99f6c-refs/branch-heads/3683@{#413}
OS Mac OS X

CCing @brave/legacy_qa in case they run into the above issue while going through 0.61.x.

@kjozwiak
Copy link
Member

kjozwiak commented Feb 26, 2019

I personally think due to the above, we should make this a higher priority. It's labelled as a refactor so not 100% sure how complicated the fix would be but this is pretty bad if someone attempts to tip multiple times in a short period of time. CCing @rebron

@btlechowski
Copy link

btlechowski commented Feb 28, 2019

This is an annoying issue. I ran into it when testing 0.60.47 and 0.61.36 on Windows 7.

@NejcZdovc Can you increase the priority for this one? P5 seems too low.

STR used:

  1. Clean install
  2. Enable rewards and claim grant
  3. Open https://kjozwiak.github.io/
  4. Quickly tip the site few times in a row (I tried 5 times)

3393

@kjozwiak kjozwiak added priority/P3 The next thing for us to work on. It'll ride the trains. and removed priority/P5 Not scheduled. Don't anticipate work on this any time soon. labels Feb 28, 2019
@kjozwiak
Copy link
Member

kjozwiak commented Feb 28, 2019

@btlechowski agreed, changed it to P3. We should probably fix this sooner than later as it's pretty obvious/annoying in 0.61.x. It was less obvious in 0.60.x. From the looks of the label, this might require a large refactor which at this point, might be to late for 0.61.x.

@NejcZdovc how big of a refactor would this be? CCing @rebron

@NejcZdovc
Copy link
Contributor

@kjozwiak this was assigned to @yrliou and she already has PR up and ready. This will be fixed really soon

@NejcZdovc NejcZdovc moved this from P3, P4, & P5 Backlog to Pending review in Rewards Feb 28, 2019
@btlechowski
Copy link

btlechowski commented Mar 5, 2019

Awesome job @yrliou. Works much better.

Verification passed on

Brave 0.61.45 Chromium: 73.0.3683.39 (Official Build) beta (64-bit)
Revision cc53b0e12fcaf42e4bab8d6c23bd4fb7aae99f6c-refs/branch-heads/3683@{#413}
OS Windows 7 Service Pack 1 Build 7601.24312

Used test plan from brave/brave-core#1800 and #3393 (comment)
There is still small delay (less than a second), but other than that works OK.
Tested on staging server.

Verification PASSED on macOS 10.14.3 x64 using the following build:

Brave 0.61.45 Chromium: 73.0.3683.39 (Official Build) beta(64-bit)
Revision cc53b0e12fcaf42e4bab8d6c23bd4fb7aae99f6c-refs/branch-heads/3683@{#413}
OS Mac OS X

Verification passed on

Brave 0.61.50 Chromium: 73.0.3683.67 (Official Build) (64-bit)
Revision a83fd4f3207ae83412d329a9ca1239dd1e068345-refs/branch-heads/3683@{#760}
OS Linux

@kjozwiak
Copy link
Member

kjozwiak commented Mar 7, 2019

@jasonrsadler if you have the time, it would be really helpful if you can see if your case is still reproducible using the latest 0.61.x.

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