-
Notifications
You must be signed in to change notification settings - Fork 2
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
Debug failures #23
Debug failures #23
Conversation
pipeline:run |
8 similar comments
pipeline:run |
pipeline:run |
pipeline:run |
pipeline:run |
pipeline:run |
pipeline:run |
pipeline:run |
pipeline:run |
pipelinee:run |
pipeline:run |
8c81c59
to
e286330
Compare
pipeline:run |
…re each test execution
f1b725d
to
98af287
Compare
98af287
to
87a2d38
Compare
Quality Gate passedThe SonarCloud Quality Gate passed, but some issues were introduced. 3 New issues |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
The changes on this PR fix the issues we have had recently with executions failing more than normal.
It turns out that sometimes when we send a transaction to the rsk or bitcoin network, the transaction can take a bit longer to reach those blockchain's mempool. Before, we were simply mining a block right after sending the transaction, thinking that it was immediately ready to be mined, but some times it is not and we didn't mine it, hence, some assertions fail.
Now, the new logic on this pr simply waits for new transactions or a specific transaction to get to the bitcoin and rsk mempool before mining.
The idea is that after sending any transaction to the bitcoin and rsk networks, before proceeding to mine and continue, we should allow enough time for the tx to get to the mempool first. This doesn't happen all the time, and usually depends on the resources that the computer running these tests have, since there are many services running independently at once while running the tests.
The
waitAndUpdateBridge
function was moved from the2wp.utils.js
file to thersk-utils.js
file to avoid circular dependency since it is now required in thersk-utils.js
file and the2wp.utils.js
is already importing thersk-utils.js
module.The
retryWithCheck
function was refactored to return an option with the result and the current attempts where it found (or not) the result. This can be useful for debugging.This pr includes the changes of the pr #20. So, once this is merged, we can close the other.