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

fix(swap): allow one extrac block to be created during swap #743

Merged
merged 2 commits into from
Dec 12, 2018
Merged

fix(swap): allow one extrac block to be created during swap #743

merged 2 commits into from
Dec 12, 2018

Conversation

offerm
Copy link
Contributor

@offerm offerm commented Dec 10, 2018

No description provided.

@ghost ghost assigned offerm Dec 10, 2018
@ghost ghost added the in progress label Dec 10, 2018
sangaman
sangaman approved these changes Dec 11, 2018
View reviewed changes
Copy link
Collaborator

@sangaman sangaman left a comment

Choose a reason for hiding this comment

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

This looks fine to me, but I'm wondering if there's a more generic approach to handling blocks during the swap. What if there are 2 new blocks, for example? Should we maybe make note of the height at the time of the swap request?

lib/swaps/Swaps.ts Show resolved Hide resolved
@offerm
Copy link
Contributor Author

offerm commented Dec 12, 2018

I will continue thinking about this. Not simple.
Still, 2 new blocks (same net) during the swap is very unlikely.

@sangaman Lets accept it as is since I need this for the trading tests.

Note that I added ceil in the ctlv calculation to avoid fraction (have seen this in action).

@sangaman
Copy link
Collaborator

Alright sounds good for now, thanks.

@sangaman sangaman merged commit 86b6d26 into master Dec 12, 2018
@ghost ghost removed the in progress label Dec 12, 2018
@sangaman sangaman deleted the cltvDelta-add-one-block branch December 12, 2018 22:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sangaman sangaman sangaman approved these changes

Assignees

@offerm offerm

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@offerm @sangaman