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

Fix (?) intermittent error in create-data script #4166

Merged
merged 1 commit into from
Jan 28, 2025

Conversation

area
Copy link
Member

@area area commented Jan 23, 2025

In #4150, I saw @mmioana had experienced one of my most hated things while trying to run the create-data script: an intermittent error

Screenshot 2025-01-23 at 10 24 36

so I went hunting and think I've arrived at a simple fix.

Currently, when the script is run:

  • Colonies get between 1 and 100 million of a token
  • Domains get between 100 and 500 of a token

So far, so good.

Individual users are then paid out from each domain by calling userPayments with the three walletUsers (which are the first three ganache accounts) and a random contributor address.

Users are paid out a random amount between 1 and 3 tokens... unless we are considering the first 'ganache' address, which gets 50 tokens, a fixed amount.

So, how does this result in a (rare) error when running the script? When paying out from a domain, the random contributor selected can be the first ganache address (if the domain is not the first in the colony to be considered). In that case, we will pay out to the first ganache address twice, resuling in somewhere between 102 and 106 tokens trying to leave the domain - but there is a chance that the domain only received as low as 100 tokens, and will not have enough to pay out.

In that scenario, the transaction will fail with the overflow message seen in the original error - but to occur, the domain must have received a sufficiently low number of tokens (less than 1% chance) and the first ganache account must have been randomly selected as a contributor (haven't properly understood how this is generated, but must be less than 33%), explaining why it is so rare to occur, anecdotally.

To fix the issue, the 'constant' amount we pay out to the first ganache address has been reduced to 30 tokens, which means that the most we can pay out from a domain is 66 tokens, which is less than the smallest amount that can be given to a domain.

For anyone paying particularly close attention, the last non-error log in the screenshot was paying out address 0x27fF0C145E191C22C75cD123C679C3e1F58a4469, which is ganacheAccounts[2], not ganacheAccounts[0]. However, the log occurs in the script after the payout succeeds, and so the error will have occurred when paying out the next address, which you can see in the failed transaction blob is 0xb77D57F4959eAfA0339424b83FcFaf9c15407461, as would be the case in the situation I've described here.

I've checked with Raul and this 50 tokens isn't a magic value anywhere, so shouldn't break anything. Obviously the issue with an intermittent error is that it's tough to be sure that it's fixed, especially one with such a low recurrence rate, but I hope that the above case is compelling.

If/when this is merged, if you see a similar error in the future, please let me know - I really detest intermittent errors, which make it incredibly hard to debug actual issues, and try to extinguish them with extreme prejudice!

@area area requested a review from a team as a code owner January 23, 2025 12:23
@rdig
Copy link
Member

rdig commented Jan 23, 2025

Screenshot from 2025-01-23 14-31-45

Very nice @area 💯

@rdig rdig assigned rdig and area and unassigned rdig Jan 24, 2025
@rdig rdig added the bug Something isn't working label Jan 24, 2025
Copy link
Member

@rdig rdig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nicely done Alex! This will alleviate that spotty error that we were seeing!

Thank you for your contribution!

Screenshot from 2025-01-24 11-17-15

Copy link
Contributor

@iamsamgibbs iamsamgibbs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice investigation! Tested just to be safe and script passes as expected and everything is still working.

Screenshot 2025-01-24 at 12 55 12

Copy link
Contributor

@mmioana mmioana left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome @area thanks so much for investigating and fixing this issue 🤩

Screenshot 2025-01-27 at 17 29 47

@rdig rdig merged commit dc22d82 into master Jan 28, 2025
2 checks passed
@rdig rdig deleted the fix/create-data-intermittent branch January 28, 2025 22:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants