Part 2: RFQ uses fixed-points #1141
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ffranr
force-pushed
the
rfq-fixed-point-part-2
branch
from
9b6917f to
3433947
Compare
Pull Request Test Coverage Report for Build 11350535264Details
💛 - Coveralls |
ffranr
force-pushed
the
rfq-fixed-point-part-2
branch
from
3433947 to
205adbe
Compare
ffranr
force-pushed
the
rfq-uses-fixed-point
branch
from
aa53ed4 to
c5a94c3
Compare
ffranr
force-pushed
the
rfq-uses-fixed-point
branch
from
c5a94c3 to
8825621
Compare
ffranr
force-pushed
the
rfq-fixed-point-part-2
branch
from
205adbe to
0142f31
Compare
|
In the latest commits pushed to this PR: RFQ wire message uses |
|
@GeorgeTsagk @guggero |
guggero
reviewed
Oct 16, 2024
ffranr
force-pushed
the
rfq-uses-fixed-point
branch
3 times, most recently
from
3a37e87 to
eeeb198
Compare
This commit modifies the AssetPurchasePolicy struct to carry the asset-to-BTC rate instead of a "price". It also updates the policy check equations to use the new rate field.
Introduce `BigInt.Bytes` and `BigInt.FromBytes` methods for encoding and decoding `BigInt` in RFQ wire messages.
This will be used for serializing fixed-points in wire messages.
ffranr
force-pushed
the
rfq-fixed-point-part-2
branch
from
0142f31 to
a6e3c35
Compare
ffranr
changed the base branch from
rfq-uses-fixed-point
to
approved-rfq-fixedpoint
|
I've rebased this PR on to the merged approved part 1 PR approved-rfq-fixedpoint. |
This commit updates the RFQ request wire message, renaming the `SuggestedRateTick` field to `SuggestedAssetRate` and changing its type to a fixed-point representation.
This commit replaces the RFQ accept wire message fields InOutRateTick and OutInRateTick with InAssetRate and OutAssetRate. The new fields represent the incoming asset-to-BTC rate and the outgoing asset-to-BTC rate, respectively.
Modify TLV type integers to match the latest BLIP state.
Update the minimum supported accept wire message version to 1. Adjust the version check logic to ensure that only version 1 is accepted, and not any version greater than 0.
ffranr
force-pushed
the
rfq-fixed-point-part-2
branch
from
a6e3c35 to
ba84cfc
Compare
Update TLV type integers to align with the latest BLIP state. Some integer values are intentionally skipped to reserve space for new fields defined in the BLIP.
Update the minimum supported request wire message version to 1. Adjust the version check logic to ensure that only version 1 is accepted, and not any version greater than 0.
ffranr
force-pushed
the
rfq-fixed-point-part-2
branch
from
ba84cfc to
a14d21d
Compare
Roasbeef
approved these changes
Oct 18, 2024
| // IntoBigIntFixedPoint converts the TlvFixedPoint to a BigIntFixedPoint. | ||
| func (f *TlvFixedPoint) IntoBigIntFixedPoint() rfqmath.BigIntFixedPoint { | ||
| return rfqmath.BigIntFixedPoint{ | ||
| Coefficient: f.fp.Coefficient, |
There was a problem hiding this comment.
Should we explicitly copy over the Coefficient here?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Depends on #1136.
Merge #1136 first. This PR will be in draft until then.
The high-level goal of this change is to represent asset-to-BTC conversion rates using the new fixed-point number type. This PR updates several "price" related fields that were previously typed as milli-satoshis, which were used as a workaround in the Proof of Concept (PoC).
Remaining changes after this PR has been merged:
suggestedOutAssetRateandtransferType.