Skip to content

Conversation

@bhandras
Copy link
Member

@bhandras bhandras commented Apr 29, 2025

Pull Request Checklist

  • Update release_notes.md if your PR contains major features, breaking changes or bugfixes

@bhandras bhandras force-pushed the autoloop-slow-by-default branch from 0c128e7 to 693d3a4 Compare April 29, 2025 16:56
Copy link
Member

@sputn1ck sputn1ck left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for the quick fix

initiator += "-" + assetSwap.assetID
}

var swapPublicationDeadline time.Time
Copy link
Member

Choose a reason for hiding this comment

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

I think this should be initialized as time.Now(), as now if you have set FastSwapPublication it will just be time.Time{}

Copy link
Member Author

Choose a reason for hiding this comment

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

I think the end-result is the same, we just want something that's presumably in the past. With that in mind the value should not matter and can just be empty for simplicity. wdyt?

Copy link
Member

Choose a reason for hiding this comment

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

That makes sense, thanks for the explanationj

Copy link
Collaborator

@hieblmi hieblmi left a comment

Choose a reason for hiding this comment

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

Looks good. But shouldn't we preserve the old default behavior of fast swaps and add the defaultPublicationDeadline as the configurable option, just so to not interrupt expectations of current auto-loop users?

@bhandras bhandras merged commit a889d62 into master Apr 30, 2025
4 checks passed
@bhandras bhandras deleted the autoloop-slow-by-default branch April 30, 2025 07:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants