Skip to content

Conversation

@notoriaga
Copy link
Contributor

Description

@swift-nav/devinfra

Fixes the signature field of MsgCertificateChain. Also makes a separate type for the signature because we use in it multiple places

API compatibility

Does this change introduce a API compatibility risk?

New messages were made for MsgCertificateChain and MsgEcdsaSignature

@notoriaga notoriaga requested review from a team and silverjam as code owners March 17, 2023 20:36
@notoriaga notoriaga requested a review from Blast545 March 17, 2023 21:03
- ECDSASignature:
short_desc: ECDSA signature
fields:
- len:
Copy link
Contributor

@silverjam silverjam Mar 17, 2023

Choose a reason for hiding this comment

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

I think if we move len field to after data then we technically would not need to rev MSG_ECDSA_SIGNATURE since the wire format/order/length of the bytes would be the same, but not sure that the message checker in CI would be able to handle that.

@silverjam
Copy link
Contributor

silverjam commented Mar 17, 2023

FWIW I think revving the messages is fine to fix this, I think once all of these signing messages are solidified we should "elide" the messages so there isn't so much cruft in the YAML spec.

E.g.

  - MSG_SIGNING_RESERVED_0:  # Was previously MSG_ECDSA_SIGNATURE_DEP_B
      id: 0x0C07
      short_desc: Reserved
      desc: Reserved
      fields:
        - reserved:
            type: array
            fill: u8
            desc: Rererved

Co-authored-by: Jason Mobarak <jason@swift-nav.com>
@notoriaga notoriaga merged commit c9af3c3 into master Mar 17, 2023
@notoriaga notoriaga deleted the steve/signing3 branch March 17, 2023 22:55
Blast545 pushed a commit that referenced this pull request Apr 26, 2023
Co-authored-by: Jason Mobarak <jason@swift-nav.com>
lkloh pushed a commit that referenced this pull request Apr 27, 2023
* update MSG_ACKNOWLEDGE on demand fields (#1303)

* variable length ecdsa signature [GV2-193] (#1306)

* Add signature length field to MsgEcdsaSignature

Co-authored-by: Jason Mobarak <jason@swift-nav.com>

* fix signature in MsgCertificateChain [GV2-193]  (#1307)

Co-authored-by: Jason Mobarak <jason@swift-nav.com>

* benchmark: run on consistent machine type

Benchmarks seem to oscilate between two values, so attempt to compensate
for this by pinning the machine type that things are run on.

* python: add framer msg type filter (#1321)

* python: add msg type filter

To help with speed issues around Python object construction, add an easy
means of avoiding work in the Python API.

* Update python/sbp/client/framer.py

Co-authored-by: Patrick Crumley <patrick.crumley@gmail.com>

* docker: use buster, stretch is archived

* python: add unit test for framer filter

* python: add unit test for framer message filter

---------

Co-authored-by: Patrick Crumley <patrick.crumley@gmail.com>

---------

Co-authored-by: Steven Meyer <smeyer@fastmail.com>
Co-authored-by: Jason Mobarak <jason@swift-nav.com>
Co-authored-by: Patrick Crumley <patrick.crumley@gmail.com>
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.

3 participants