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

Implement workaround for SortedTroves edge case #659

Closed
danielattilasimon opened this issue Jul 30, 2021 · 1 comment · Fixed by #663
Closed

Implement workaround for SortedTroves edge case #659

danielattilasimon opened this issue Jul 30, 2021 · 1 comment · Fixed by #663
Assignees
Labels
bug Something isn't working middleware released

Comments

@danielattilasimon
Copy link
Collaborator

This relates to the backend quirk outlined by #600, which has been partially addressed by e469beb, however since then another edge case has been discovered.

As @RickGriff noted:

We just had a user who had the lowest ICR trove in the system, trying to adjust his trove to go lower. The tx failed before broadcast - presumably because gas estimation didn't allow it.

I believe this is the edge case DeFiSaver found - and I mistakenly thought the solution outlined above also covered it. The problem is that for the last trove in the list, the addressOfLowestICRTrove is their own address - and the reinsertion first removes it before the insert. The system can't find it in the list, so starts at the opposite end, and runs out of gas.

The DefiSaver fix was to check for this specific edge case: if the trove is last in the list, use the 2nd last trove's address as both hints.

We should implement the same workaround in the SDK.

@github-actions
Copy link

🎉 This issue has been resolved in version @liquity/lib-ethers-v3.3.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working middleware released
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant