search

arthurlm / quickfix-rs

Unofficial rust binding of QuickFIX library
https://docs.rs/quickfix/latest/quickfix/
Other
13 stars 5 forks source link

Add Long field type #15

Closed blerstt closed 7 months ago

blerstt commented 7 months ago

Changing RptSeq field type from LONG to INT in this commit d068ae4 causes Quickfix to send reject messages while encountering 83 field, for example:

"8=FIXT.1.1|9=148|35=3|34=527|49=KEY|52=20240304-13:42:39.528|56=Coinbase|45=15023|58=Incorrect data format for value|371=83|372=W|373=6|10=054|"

I have added FieldType::Long as i128 to aid the issue and now program is operating as normal.

codecov[bot] commented 7 months ago

Codecov Report

Attention: Patch coverage is 33.33333% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 92.38%. Comparing base (2cc3304) to head (a857247).

Files Patch % Lines
quickfix-msg-gen/src/lib.rs 50.00% 1 Missing and 1 partial :warning:
quickfix-spec-parser/src/model/field_type.rs 0.00% 1 Missing and 1 partial :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #15 +/- ## ========================================== - Coverage 92.51% 92.38% -0.13% ========================================== Files 37 37 Lines 2098 2102 +4 Branches 2098 2102 +4 ========================================== + Hits 1941 1942 +1 - Misses 86 88 +2 - Partials 71 72 +1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

arthurlm commented 7 months ago

Thanks a lot for the fix. I have also revert my commit where I have badly change Coinbase RptSeq type.

Feel free to re-open / create new issue in case something else is wrong.

Do you want me to publish a new release for the message generator crate ? Or you can work with git URL in your Cargo.toml file (so I can wait until there is no more hidden issues on this) ?

blerstt commented 7 months ago

I can wait, I will use my git in the meantime.

arthurlm commented 7 months ago

Just a side note on what is happening behind the scene with LONG type.

After thinking a little bit more on this, I was surprise it actually works. So I check the C++ code of the quickfix library and here is my guess on what is going on:

  1. When constructing DataDictionary, readFromStream then readFromDocument functions are called.
  2. For each field, type is save when calling addFieldType.
  3. XML type to quickfix internal type is converted in DataDictionary::XMLTypeToType.

⚠️ Here you can check that LONG is not in the list, so quickfix will fallback to TYPE::Unknown.

  1. When validating messages checkValidFormat is called for each field.
  2. Since type is unknown no check will be done (see code here)

I guess you can try replacing LONG by FOOBAR is the XML spec file, and quickfix engine will also validate messages.

I will not revert anything since I still believe this PR can be useful, but be aware that no check will be done by quickfix when validating messages from coinbase. All checks will be done on Rust side when parsing String to i128.