Backward compat support for Transaction.sym: Symbol
#477
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
In order to support existing `pps.toml` files in the wild which don't have the `asset_type, price_tick_size, lot_tick_size` fields, we need to only optionally read them and instead expect that backends will write the fields going forward (coming in follow patches). Further this makes some small asset-size (vlm accounting) quantization related adjustments: - rename `Symbol.decimal_quant()` -> `.quantize_size()` since that is explicitly what this method is doing. - and expect an input `size: float` which we cast to decimal instead of doing it inside the `.calc_size()` caller code. - drop `Symbol.iterfqsns()` which wasn't being used anywhere at all.. Additionally, this drafts out a new replacement market-trading-pair data type to eventually replace `.data._source.Symbol` -> `MktPair` which we aren't using yet, but serves as the documentation-driven motivator ;) and, it relates to #467.
Also includes a retyping of `Client._pair: dict[str, Pair]` to look up pair structs and map all alt-key-name-sets to each for easy precision info lookup to set the `.sym` field for each transaction including for on-chain transfers which kraken provides as an "asset decimals" field, presumably pulled from the particular block-token's limitation info.
goodboy
commented
Mar 3, 2023
| @@ -127,6 +134,56 @@ def unpack_fqsn(fqsn: str) -> tuple[str, str, str]: | |||
| ) | |||
|
|
|||
|
|
|||
| class MktPair(Struct, frozen=True): | |||
There was a problem hiding this comment.
Wrt this, i think if we want to have multiple endpoint brokers addressable eventually we can have type that maybe groups this type into some sort of address collection of sorts?
guilledk
approved these changes
Mar 3, 2023
This was referenced Mar 5, 2023
Merged
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.
Like it sounds makes #470 still work on existing
pps.tomlfiles in the wild 😉Further this ports the
krakenandibbackends to correctly provide the precision info for transaction sizes where possible by pulling the info from the market info available either from ledger or API records.Additionally, this includes a draft implementation for a
.data._source.Symbolreplacement:MktPair.It's not used at all but gives us a guide for how we could majorly simplify the interface and provide a much clearer defined market addressing system especially as it pertains to refining our schema as documented in #467.