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.

Sign up for GitHub

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

Jump to bottom

add retries to mobilecoind when it submits transactions #3308

Merged
merged 6 commits into from
Apr 11, 2023
Merged

add retries to mobilecoind when it submits transactions #3308

merged 6 commits into from
Apr 11, 2023

Conversation

cbeck88
Copy link
Contributor

@cbeck88 cbeck88 commented Apr 10, 2023

i'm finding that sometimes it fails with attestation permission denied errors otherwise

feel free to leave comments about how you would like this to be more configurable or use exponential backoff or something, let's decide exactly how it should work during review

@cbeck88
add retries to mobilecoind when it submits transactions
258ba20 
i'm finding that sometimes it fails with attestation permission
denied errors otherwise

---

feel free to leave comments about how you would like this to be
more configurable or use exponential backoff or something, let's
decide exactly how it should work during review
@cbeck88 cbeck88 requested review from eranrund, a team, NotGyro, nick-mobilecoin and jcape and removed request for a team April 10, 2023 20:28
@cbeck88 cbeck88 self-assigned this Apr 10, 2023
@cbeck88 cbeck88 added mobilecoind The MobileCoinD application bug Something isn't working labels Apr 10, 2023
@cbeck88
Copy link
Contributor Author

cbeck88 commented Apr 10, 2023 •
edited
Loading

i decided the most sensible thing is to just make it match what full-service does, since that seems to be working in prod

https://github.com/mobilecoinofficial/full-service/blob/0868e6db0e7ca066507c45c8af0b00f67587d3be/full-service/src/service/transaction.rs#L554

nick-mobilecoin
nick-mobilecoin requested changes Apr 11, 2023
View reviewed changes
mobilecoind/src/payments.rs Outdated Show resolved Hide resolved
@cbeck88 cbeck88 requested a review from jgreat April 11, 2023 14:44
jcape
jcape reviewed Apr 11, 2023
View reviewed changes
mobilecoind/src/payments.rs Outdated Show resolved Hide resolved
jgreat
jgreat approved these changes Apr 11, 2023
View reviewed changes
Copy link
Contributor

@jgreat jgreat left a comment

Choose a reason for hiding this comment

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

I misunderstood the conversation here. This is adding in some kind of retry where we didn't have any defined. We should open an issue to follow up and optimize how we are doing retries.

@cbeck88 cbeck88 merged commit e9f4280 into master Apr 11, 2023
@cbeck88 cbeck88 deleted the add-retries-to-mobilecoind-submits branch April 11, 2023 18:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jcape jcape jcape left review comments

@nick-mobilecoin nick-mobilecoin nick-mobilecoin approved these changes

@jgreat jgreat jgreat approved these changes

@eranrund eranrund Awaiting requested review from eranrund

@NotGyro NotGyro Awaiting requested review from NotGyro NotGyro was automatically assigned from mobilecoinfoundation/coredev

Assignees

@cbeck88 cbeck88

Labels
bug Something isn't working mobilecoind The MobileCoinD application
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@cbeck88 @jcape @jgreat @nick-mobilecoin