Skip to content

feat(core)!: lowerCamelCase for protobuf json mapping#1250

Merged
SuperFluffy merged 1 commit intomainfrom
superfluffy/correct-pbjson-field-case
Jul 9, 2024
Merged

feat(core)!: lowerCamelCase for protobuf json mapping#1250
SuperFluffy merged 1 commit intomainfrom
superfluffy/correct-pbjson-field-case

Conversation

@SuperFluffy
Copy link
Contributor

Summary

Use lowerCamelCase for protobuf message field names mapped to JSON object keys.

Background

The Protobuf to JSON mapping prescribes that protobuf message fields must use lowerCamelCase when mapped to JSON object keys. The protobuf-compiler tool was erroneously preserving the Rust field names which re snake_case.

Changes

  • Remove pbson_build::Builder::preserve_proto_field_names from its build chain
  • Regenerate serde Serialize and Deserialize impls for all protobuf Rust types
  • Fix Conductor test mocks to use the correct case

Testing

All tests still run.

Breaking Changelist

This is neither network breaking nor service breaking because the Astria stack only uses protobuf (not its JSON counterparts) on the wire. However, this is still marked as breaking due to external consumers being affected.

conductor: fix re-panicking and expected mock requests
@github-actions github-actions bot added conductor pertaining to the astria-conductor crate proto pertaining to the Astria Protobuf spec labels Jul 9, 2024
@SuperFluffy SuperFluffy marked this pull request as ready for review July 9, 2024 09:41
@SuperFluffy SuperFluffy requested a review from a team as a code owner July 9, 2024 09:41
@SuperFluffy SuperFluffy requested a review from noot July 9, 2024 09:41
@SuperFluffy SuperFluffy added this pull request to the merge queue Jul 9, 2024
Merged via the queue into main with commit f69306f Jul 9, 2024
@SuperFluffy SuperFluffy deleted the superfluffy/correct-pbjson-field-case branch July 9, 2024 12:42
steezeburger added a commit that referenced this pull request Jul 15, 2024
* main:
  feat(cli): add cmd to collect withdrawal events and submit as actions (#1261)
  fix(core, bridge, sequencer)!: dismabiguate return addresses (#1266)
  fix(withdrawer): support withdrawer address that differs from bridge address   (#1262)
  (core, sequencer)!: generate serde traits impls for all protocol protobufs (#1260)
  fix(charts): add resources for sequencer/cometbft (#1254)
  chore(sequencer)!: add metrics (#1248)
  fix(sequencer-utils): fixes issue in `parse_blob` tests (#1243)
  feat(core, proto)!: make bridge unlock memo string (#1244)
  fix(conductor): don't panic during panic (#1252)
  feat(core)!: lowerCamelCase for protobuf json mapping (#1250)
  refactor(bridge-withdrawer)!: refactor startup to a separate subtask and remove balance check from startup (#1190)
  fix: rollup archive node configurations (#1249)
  refactor(core, bridge-withdrawer)!: move bridge-unlock memo to core (#1245)
  fix(sequencer)!: store native asset ibc->trace mapping in init_chain (#1242)
steezeburger added a commit that referenced this pull request Jul 19, 2024
* main: (24 commits)
  chore: update `bytes` and `ics23` crates (#1279)
  fix(sequencer): improve and fix instrumentation (#1255)
  feature(charts): hermes chart fixes, bech32 updates, ibc bridge test (#1130)
  chore(cli): remove unused rollup cli code (#1275)
  chore(test): use a temporary file to not pollute the workspace (#1269)
  chore(sequencer): add mempool benchmarks (#1238)
  fix(bridge-withdrawer)!: fix nonce handling (#1215)
  feat(cli, bridge-withdrawer)!: share code between cli and service (#1270)
  feat(cli): add cmd to collect withdrawal events and submit as actions (#1261)
  fix(core, bridge, sequencer)!: dismabiguate return addresses (#1266)
  fix(withdrawer): support withdrawer address that differs from bridge address   (#1262)
  (core, sequencer)!: generate serde traits impls for all protocol protobufs (#1260)
  fix(charts): add resources for sequencer/cometbft (#1254)
  chore(sequencer)!: add metrics (#1248)
  fix(sequencer-utils): fixes issue in `parse_blob` tests (#1243)
  feat(core, proto)!: make bridge unlock memo string (#1244)
  fix(conductor): don't panic during panic (#1252)
  feat(core)!: lowerCamelCase for protobuf json mapping (#1250)
  refactor(bridge-withdrawer)!: refactor startup to a separate subtask and remove balance check from startup (#1190)
  fix: rollup archive node configurations (#1249)
  ...
bharath-123 pushed a commit that referenced this pull request Jul 25, 2024
## Summary
Use `lowerCamelCase` for protobuf message field names mapped to JSON
object keys.

## Background
The Protobuf to JSON mapping prescribes that protobuf message fields
must use `lowerCamelCase` when mapped to JSON object keys. The
protobuf-compiler tool was erroneously preserving the Rust field names
which re `snake_case`.

## Changes
- Remove `pbson_build::Builder::preserve_proto_field_names` from its
build chain
- Regenerate serde `Serialize` and `Deserialize` impls for all protobuf
Rust types
- Fix Conductor test mocks to use the correct case

## Testing
All tests still run.

## Breaking Changelist
This is neither network breaking nor service breaking because the Astria
stack only uses protobuf (not its JSON counterparts) on the wire.
However, this is still marked as breaking due to external consumers
being affected.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

conductor pertaining to the astria-conductor crate proto pertaining to the Astria Protobuf spec

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants