Skip to content

apollo_consensus_orchestrator: add Deserialize for testing#13945

Merged
dorimedini-starkware merged 1 commit intomainfrom
04-30-apollo_consensus_orchestrator_add_deserialize_for_testing
May 3, 2026
Merged

apollo_consensus_orchestrator: add Deserialize for testing#13945
dorimedini-starkware merged 1 commit intomainfrom
04-30-apollo_consensus_orchestrator_add_deserialize_for_testing

Conversation

@dorimedini-starkware
Copy link
Copy Markdown
Collaborator

No description provided.

@reviewable-StarkWare
Copy link
Copy Markdown

This change is Reviewable

Copy link
Copy Markdown
Collaborator Author

dorimedini-starkware commented May 3, 2026

@cursor
Copy link
Copy Markdown

cursor Bot commented May 3, 2026

PR Summary

Low Risk
Low risk: changes are gated behind testing/test cfgs and only add Deserialize/PartialEq derives plus feature wiring, without modifying runtime behavior or data formats in production builds.

Overview
Adds a testing feature path to allow deserializing and comparing Aerospike/CENDE “central objects” in tests. This wires apollo_consensus_orchestrator’s testing feature to shared_execution_objects and conditionally derives serde::Deserialize/PartialEq on AerospikeBlob, FeeMarketInfo, and the various Central* structs/enums used for recorder payloads.

In shared_execution_objects, introduces a testing feature and makes CentralTransactionExecutionInfo derive PartialEq under it to simplify test assertions.

Reviewed by Cursor Bugbot for commit dd4db61. Bugbot is set up for automated code reviews on this repo. Configure here.

@dorimedini-starkware dorimedini-starkware force-pushed the 04-30-apollo_consensus_orchestrator_add_deserialize_for_testing branch from 1c25347 to 5707677 Compare May 3, 2026 10:07
@dorimedini-starkware dorimedini-starkware force-pushed the 04-30-apollo_consensus_orchestrator_add_deserialize_for_testing branch from 5707677 to dd4db61 Compare May 3, 2026 10:25
Copy link
Copy Markdown
Contributor

@yoavGrs yoavGrs left a comment

Choose a reason for hiding this comment

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

@yoavGrs made 1 comment.
Reviewable status: 0 of 6 files reviewed, 1 unresolved discussion (waiting on dorimedini-starkware and matanl-starkware).


a discussion (no related file):
In general, why is deserialization gated? For heavy objects?

Copy link
Copy Markdown
Collaborator Author

@dorimedini-starkware dorimedini-starkware left a comment

Choose a reason for hiding this comment

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

@dorimedini-starkware made 1 comment.
Reviewable status: 0 of 6 files reviewed, 1 unresolved discussion (waiting on matanl-starkware and yoavGrs).


a discussion (no related file):

Previously, yoavGrs wrote…

In general, why is deserialization gated? For heavy objects?

because it's only needed in my test. if it ever has a real use case, we can remove the gate

Copy link
Copy Markdown
Contributor

@yoavGrs yoavGrs left a comment

Choose a reason for hiding this comment

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

:lgtm:

@yoavGrs reviewed 6 files and all commit messages, made 1 comment, and resolved 1 discussion.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on matanl-starkware).

@dorimedini-starkware dorimedini-starkware added this pull request to the merge queue May 3, 2026
Merged via the queue into main with commit 8b6b28d May 3, 2026
18 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators May 5, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants