Skip to content
This repository has been archived by the owner on Jan 22, 2025. It is now read-only.

Add serialization-control enum for RPC Options #27676

Merged
merged 11 commits into from
Sep 14, 2022

Conversation

CriesofCarrots
Copy link
Contributor

Problem

As per #27601 (comment), it would be helpful to have complete control over the serialization of optional fields for JSON-RPC responses.

Summary of Changes

Add OptionSerializer enum and custom Serialize implementation to support skipping, serializing the type default, and serializing None for the same field in different contexts.

@CriesofCarrots
Copy link
Contributor Author

CriesofCarrots commented Sep 8, 2022

@jstarry can you please take a look? Interested in feedback on direction and the api interface, as well as any bikeshedding. I used the new enum for a couple fields in UiTransactionStatusMeta to demonstrate the api, but can flesh out that whole struct before merge.

@CriesofCarrots CriesofCarrots force-pushed the tx-meta-serde branch 3 times, most recently from 1877241 to e655277 Compare September 8, 2022 23:48
transaction-status/src/option_serializer.rs Outdated Show resolved Hide resolved
transaction-status/src/option_serializer.rs Outdated Show resolved Hide resolved
transaction-status/src/lib.rs Show resolved Hide resolved
@CriesofCarrots CriesofCarrots force-pushed the tx-meta-serde branch 4 times, most recently from 09d2174 to aeb16ee Compare September 14, 2022 01:36
@CriesofCarrots CriesofCarrots marked this pull request as ready for review September 14, 2022 01:48
transaction-status/src/lib.rs Outdated Show resolved Hide resolved
transaction-status/src/lib.rs Outdated Show resolved Hide resolved
transaction-status/src/lib.rs Outdated Show resolved Hide resolved
transaction-status/src/lib.rs Outdated Show resolved Hide resolved
transaction-status/src/option_serializer.rs Outdated Show resolved Hide resolved
transaction-status/src/option_serializer.rs Outdated Show resolved Hide resolved
transaction-status/src/option_serializer.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@jstarry jstarry left a comment

Choose a reason for hiding this comment

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

Can you please add tests which enforce the expected json responses of relevant rpc responses?

@CriesofCarrots
Copy link
Contributor Author

CriesofCarrots commented Sep 14, 2022

Can you please add tests which enforce the expected json responses of relevant rpc responses?

Please see first two commits. The tests pass both before and after the other commits in this PR.

@CriesofCarrots CriesofCarrots merged commit 360ca07 into solana-labs:master Sep 14, 2022
mergify bot pushed a commit that referenced this pull request Sep 14, 2022
* Add test to verify output when deserializing/reserializing empty fields

* Add test to verify serialization from UiTransactionStatusMeta constructors

* Add OptionSerializer

* Use OptionSerializer for inner_instructions

* Use OptionSerializer for loaded_addresses

* Remove Default variant, use into instead

* Add as_ref implementation

* Use OptionSerializer for log_messages and rewards

* Use OptionSerializer for token_balances

* Use OptionSerializer for return_data

* Use OptionSerializer for compute_units_consumed

(cherry picked from commit 360ca07)

# Conflicts:
#	cli-output/src/display.rs
#	client/src/mock_sender.rs
#	transaction-status/src/lib.rs
mergify bot pushed a commit that referenced this pull request Sep 14, 2022
* Add test to verify output when deserializing/reserializing empty fields

* Add test to verify serialization from UiTransactionStatusMeta constructors

* Add OptionSerializer

* Use OptionSerializer for inner_instructions

* Use OptionSerializer for loaded_addresses

* Remove Default variant, use into instead

* Add as_ref implementation

* Use OptionSerializer for log_messages and rewards

* Use OptionSerializer for token_balances

* Use OptionSerializer for return_data

* Use OptionSerializer for compute_units_consumed

(cherry picked from commit 360ca07)
CriesofCarrots pushed a commit that referenced this pull request Sep 14, 2022
* Add test to verify output when deserializing/reserializing empty fields

* Add test to verify serialization from UiTransactionStatusMeta constructors

* Add OptionSerializer

* Use OptionSerializer for inner_instructions

* Use OptionSerializer for loaded_addresses

* Remove Default variant, use into instead

* Add as_ref implementation

* Use OptionSerializer for log_messages and rewards

* Use OptionSerializer for token_balances

* Use OptionSerializer for return_data

* Use OptionSerializer for compute_units_consumed

(cherry picked from commit 360ca07)
CriesofCarrots pushed a commit to CriesofCarrots/solana that referenced this pull request Sep 14, 2022
* Add test to verify output when deserializing/reserializing empty fields

* Add test to verify serialization from UiTransactionStatusMeta constructors

* Add OptionSerializer

* Use OptionSerializer for inner_instructions

* Use OptionSerializer for loaded_addresses

* Remove Default variant, use into instead

* Add as_ref implementation

* Use OptionSerializer for log_messages and rewards

* Use OptionSerializer for token_balances

* Use OptionSerializer for return_data

* Use OptionSerializer for compute_units_consumed

(cherry picked from commit 360ca07)

# Conflicts:
#	cli-output/src/display.rs
#	client/src/mock_sender.rs
#	transaction-status/src/lib.rs
mergify bot added a commit that referenced this pull request Sep 15, 2022
)

* Add serialization-control enum for RPC Options (#27676)

* Add test to verify output when deserializing/reserializing empty fields

* Add test to verify serialization from UiTransactionStatusMeta constructors

* Add OptionSerializer

* Use OptionSerializer for inner_instructions

* Use OptionSerializer for loaded_addresses

* Remove Default variant, use into instead

* Add as_ref implementation

* Use OptionSerializer for log_messages and rewards

* Use OptionSerializer for token_balances

* Use OptionSerializer for return_data

* Use OptionSerializer for compute_units_consumed

(cherry picked from commit 360ca07)

* Appease incorrect nightly lint

Co-authored-by: Tyera Eulberg <[email protected]>
mergify bot added a commit that referenced this pull request Sep 15, 2022
)

* Add serialization-control enum for RPC Options (#27676)

* Add test to verify output when deserializing/reserializing empty fields

* Add test to verify serialization from UiTransactionStatusMeta constructors

* Add OptionSerializer

* Use OptionSerializer for inner_instructions

* Use OptionSerializer for loaded_addresses

* Remove Default variant, use into instead

* Add as_ref implementation

* Use OptionSerializer for log_messages and rewards

* Use OptionSerializer for token_balances

* Use OptionSerializer for return_data

* Use OptionSerializer for compute_units_consumed

(cherry picked from commit 360ca07)

# Conflicts:
#	cli-output/src/display.rs
#	client/src/mock_sender.rs
#	transaction-status/src/lib.rs

* Fix conflicts

Co-authored-by: Tyera Eulberg <[email protected]>
CriesofCarrots pushed a commit that referenced this pull request Sep 15, 2022
)

* Add serialization-control enum for RPC Options (#27676)

* Add test to verify output when deserializing/reserializing empty fields

* Add test to verify serialization from UiTransactionStatusMeta constructors

* Add OptionSerializer

* Use OptionSerializer for inner_instructions

* Use OptionSerializer for loaded_addresses

* Remove Default variant, use into instead

* Add as_ref implementation

* Use OptionSerializer for log_messages and rewards

* Use OptionSerializer for token_balances

* Use OptionSerializer for return_data

* Use OptionSerializer for compute_units_consumed

(cherry picked from commit 360ca07)

# Conflicts:
#	cli-output/src/display.rs
#	client/src/mock_sender.rs
#	transaction-status/src/lib.rs

* Fix conflicts
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.

2 participants