fix: change protobuf definition to use repeated fields for consistency #7437
Conversation
thecockatiel
changed the title
WalkthroughThis pull request standardizes data conversion across multiple modules by replacing direct map operations with list‐based approaches. Several classes now use utility methods from the new Changes
Sequence Diagram(s)sequenceDiagram
participant Builder as Proto Builder
participant Map as ExtraDataMap
participant UT as ProtoUtil
participant ProtoMsg as Protobuf Message
Builder->>UT: Call toStringMapEntryList(extraDataMap)
UT-->>Builder: List<StringMapEntry>
Builder->>ProtoMsg: addAllExtraData(list)
sequenceDiagram
participant ProtoMsg as Protobuf Message
participant UT as ProtoUtil
participant Object as Domain Object
ProtoMsg->>UT: getExtraDataList()
UT-->>Object: Convert to Map<String,String>
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
core/src/main/java/bisq/core/payment/payload/PaymentMethod.java (1)
20-21: Consider Usage or Removal of ImportedProtoUtil.The new import of
bisq.common.proto.ProtoUtilaligns with the broader changes toward deterministic serialization by using repeated fields. However, note that it isn’t currently used anywhere in the file. If you plan to incorporate it in future enhancements (for example, to adjust how extra data is serialized), then retaining it is fine; otherwise, it might be cleaner to remove unused imports to reduce clutter.core/src/main/java/bisq/core/payment/payload/BsqSwapAccountPayload.java (1)
20-21: Imported but unused class.The
ProtoUtilclass is imported but not used anywhere in this file. Consider removing the unused import or utilizing it if needed for the protobuf serialization approach.If this import is part of a broader refactoring effort and will be used in future commits, please add a comment explaining its purpose.
core/src/main/java/bisq/core/payment/payload/IfscBasedAccountPayload.java (1)
3-4: Imported but unused class.The
ProtoUtilclass is imported but not used anywhere in this file. Consider removing the unused import or implementing the appropriate conversion methods for protobuf serialization consistency.If this import is part of a broader refactoring effort to standardize protobuf serialization across the codebase, please add a comment explaining its future usage.
core/src/main/java/bisq/core/payment/payload/CountryBasedPaymentAccountPayload.java (1)
20-21: Imported but unused class.The
ProtoUtilclass is imported but not used anywhere in this file. Consider removing the unused import or implementing the appropriate conversion methods if needed.If this import is part of a broader refactoring effort to standardize protobuf serialization across the codebase, please add a comment explaining its intended purpose.
proto/src/main/proto/pb.proto (1)
23-31: Review: StringMapEntry Message DefinitionThe new message uses a oneof field to simulate an optional string value so that the serialized bytes match a native map. This “hack” is acceptable given that older proto versions do not support the optional keyword—but please ensure this design choice is clearly documented for future maintainers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
core/src/test/resources/serialization_test_data/DaoStateDump.binis excluded by!**/*.bin
📒 Files selected for processing (99)
common/src/main/java/bisq/common/ExcludeForHashAwareProto.java(2 hunks)common/src/main/java/bisq/common/proto/ProtoUtil.java(2 hunks)common/src/main/java/bisq/common/util/Utilities.java(1 hunks)core/src/main/java/bisq/core/alert/Alert.java(3 hunks)core/src/main/java/bisq/core/dao/governance/blindvote/BlindVote.java(4 hunks)core/src/main/java/bisq/core/dao/governance/proposal/storage/temp/TempProposalPayload.java(4 hunks)core/src/main/java/bisq/core/dao/state/model/DaoState.java(4 hunks)core/src/main/java/bisq/core/dao/state/model/governance/ChangeParamProposal.java(3 hunks)core/src/main/java/bisq/core/dao/state/model/governance/CompensationProposal.java(3 hunks)core/src/main/java/bisq/core/dao/state/model/governance/ConfiscateBondProposal.java(3 hunks)core/src/main/java/bisq/core/dao/state/model/governance/Cycle.java(2 hunks)core/src/main/java/bisq/core/dao/state/model/governance/GenericProposal.java(3 hunks)core/src/main/java/bisq/core/dao/state/model/governance/Proposal.java(2 hunks)core/src/main/java/bisq/core/dao/state/model/governance/ReimbursementProposal.java(3 hunks)core/src/main/java/bisq/core/dao/state/model/governance/RemoveAssetProposal.java(3 hunks)core/src/main/java/bisq/core/dao/state/model/governance/RoleProposal.java(3 hunks)core/src/main/java/bisq/core/filter/Filter.java(3 hunks)core/src/main/java/bisq/core/network/p2p/inventory/messages/GetInventoryResponse.java(2 hunks)core/src/main/java/bisq/core/offer/bisq_v1/OfferPayload.java(2 hunks)core/src/main/java/bisq/core/offer/bsq_swap/BsqSwapOfferPayload.java(2 hunks)core/src/main/java/bisq/core/payment/PaymentAccount.java(2 hunks)core/src/main/java/bisq/core/payment/payload/AchTransferAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/AdvancedCashAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/AliPayAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/AmazonGiftCardAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/AssetsAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/AustraliaPayidAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/BankAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/BizumAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/BsqSwapAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/CapitualAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/CashAppAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/CashByMailAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/CashDepositAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/CelPayAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/ChaseQuickPayAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/ClearXchangeAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/CountryBasedPaymentAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/CryptoCurrencyAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/DomesticWireTransferAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/F2FAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/FasterPaymentsAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/HalCashAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/IfscBasedAccountPayload.java(1 hunks)core/src/main/java/bisq/core/payment/payload/ImpsAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/InstantCryptoCurrencyPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/InteracETransferAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/JapanBankAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/MercadoPagoAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/MoneseAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/MoneyBeamAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/MoneyGramAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/NationalBankAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/NeftAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/NequiAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/OKPayAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/PaxumAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/PayloadWithHolderName.java(1 hunks)core/src/main/java/bisq/core/payment/payload/PaymentAccountPayload.java(3 hunks)core/src/main/java/bisq/core/payment/payload/PaymentMethod.java(1 hunks)core/src/main/java/bisq/core/payment/payload/PayseraAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/PaytmAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/PerfectMoneyAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/PixAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/PopmoneyAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/PromptPayAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/RevolutAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/RtgsAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/SameBankAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/SatispayAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/SbpAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/SepaAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/SepaInstantAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/SpecificBanksAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/StrikeAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/SwiftAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/SwishAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/TikkieAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/TransferwiseAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/TransferwiseUsdAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/USPostalMoneyOrderAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/UpholdAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/UpiAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/VenmoAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/VerseAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/WeChatPayAccountPayload.java(2 hunks)core/src/main/java/bisq/core/payment/payload/WesternUnionAccountPayload.java(2 hunks)core/src/main/java/bisq/core/support/dispute/Dispute.java(3 hunks)core/src/main/java/bisq/core/support/dispute/arbitration/arbitrator/Arbitrator.java(3 hunks)core/src/main/java/bisq/core/support/dispute/mediation/mediator/Mediator.java(3 hunks)core/src/main/java/bisq/core/support/dispute/refund/refundagent/RefundAgent.java(3 hunks)core/src/main/java/bisq/core/trade/statistics/TradeStatistics2.java(2 hunks)core/src/main/java/bisq/core/trade/statistics/TradeStatistics3.java(2 hunks)core/src/main/java/bisq/core/user/PreferencesPayload.java(2 hunks)core/src/main/java/bisq/core/user/UserPayload.java(3 hunks)core/src/test/java/bisq/core/protobuf/SerializationTest.java(1 hunks)p2p/src/main/java/bisq/network/p2p/storage/payload/MailboxStoragePayload.java(5 hunks)proto/src/main/proto/pb.proto(28 hunks)proto/src/main/proto/pb_old.proto(1 hunks)
🧰 Additional context used
🪛 Buf (1.47.2)
proto/src/main/proto/pb_old.proto
19-19: Files with package "io.bisq.protobuffer.old" must be within a directory "io/bisq/protobuffer/old" relative to root but were in directory "proto/src/main/proto".
(PACKAGE_DIRECTORY_MATCH)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Test Java 11, macOS-latest
🔇 Additional comments (196)
common/src/main/java/bisq/common/proto/ProtoUtil.java (1)
115-138: Well-implemented utility methods for deterministic serializationThe newly added utility methods for converting between Map<String, String> and List<protobuf.StringMapEntry> are well-implemented with appropriate null/empty checks, good use of Java streams, and proper handling of duplicate keys.
Using LinkedHashMap ensures order preservation when converting back from lists to maps, which aligns well with the goal of maintaining deterministic serialization.
core/src/main/java/bisq/core/payment/payload/AssetsAccountPayload.java (1)
20-21:✅ Verification successful
Import added but not yet used
The ProtoUtil import has been added but is not currently used anywhere in this class. Based on the PR objectives about standardizing repeated fields for map serialization, this is likely in preparation for future changes.
🏁 Script executed:
#!/bin/bash # Check if ProtoUtil is actually used in this file in any derived classes rg -A 5 -B 5 "toStringMapEntryList|toStringMap" --type java core/src/main/java/bisq/core/payment/payload/AssetsAccountPayload.javaLength of output: 132
Unused
ProtoUtilImport – Prepared for Future Enhancements
The file still contains theimport bisq.common.proto.ProtoUtil;on lines 20–21, and our verification (searching for its potential usage) confirms there are currently no calls to its associated methods in this file. As noted in the PR objectives, this unused import appears to be a placeholder for upcoming changes related to standardizing map serialization. If these changes are imminent, keeping the import is acceptable; otherwise, you might consider removing it later to keep the code clean.core/src/main/java/bisq/core/payment/payload/PayloadWithHolderName.java (1)
20-21:✅ Verification successful
Import added but not yet used
The ProtoUtil import has been added but is not currently used anywhere in this interface. This is likely in preparation for future changes to standardize serialization of maps to repeated fields across classes implementing this interface.
🏁 Script executed:
#!/bin/bash # Check implementations of PayloadWithHolderName to see if any use ProtoUtil echo "Implementations of PayloadWithHolderName that might use ProtoUtil:" rg -l "implements\s+PayloadWithHolderName" --type java | xargs rg -l "ProtoUtil\.(toStringMapEntryList|toStringMap)"Length of output: 1236
Verified: ProtoUtil Import is Forward-Looking and Valid
The import of
bisq.common.proto.ProtoUtilin thePayloadWithHolderNameinterface isn’t used directly there—but verification shows that several implementing classes (such asPopmoneyAccountPayload,CashByMailAccountPayload,ClearXchangeAccountPayload, and others) already leverage ProtoUtil methods (e.g.,toStringMapEntryList/toStringMap). This confirms that the import is intentionally added in anticipation of standardizing map serialization across implementations. No refactoring is needed at this time.core/src/main/java/bisq/core/payment/payload/BankAccountPayload.java (1)
20-21:✅ Verification successful
Import added but not yet used
The ProtoUtil import has been added but is not currently used in this class. Based on the PR description about replacing maps with repeated fields for deterministic serialization, this import is likely in preparation for changes to how the excludeFromJsonDataMap is handled in getPaymentAccountPayloadBuilder().
🏁 Script executed:
#!/bin/bash # Check if this class or subclasses use any map serialization that could be converted echo "Checking BankAccountPayload and subclasses for map serialization:" rg -A 3 -B 3 "excludeFromJsonDataMap" --type java core/src/main/java/bisq/core/payment/payload/BankAccountPayload.javaLength of output: 747
Review Update: Unused ProtoUtil Import is Intentional
The
ProtoUtilimport incore/src/main/java/bisq/core/payment/payload/BankAccountPayload.javais currently unused. This aligns with the PR description—it's included in preparation for upcoming changes to replace maps with repeated fields for deterministic serialization (affecting howexcludeFromJsonDataMapis handled ingetPaymentAccountPayloadBuilder()). No changes are required at this point, but please ensure that once the new serialization approach is implemented the import becomes actively used or is otherwise removed if no longer needed.
- Location:
core/src/main/java/bisq/core/payment/payload/BankAccountPayload.java(lines 20-21)core/src/main/java/bisq/core/payment/payload/ClearXchangeAccountPayload.java (2)
20-21: New import for ProtoUtil.This import is correctly added to support the new serialization approach in the
fromProtomethod.
85-85: Map serialization approach updated for consistency.The code now correctly uses
ProtoUtil.toStringMap()to convert from a repeated field list to a map, aligning with the PR's goal of ensuring deterministic serialization of maps.core/src/main/java/bisq/core/dao/state/model/governance/Proposal.java (2)
26-26: Adding import for ProtoUtilThe import for
ProtoUtilis appropriately added to support the usage of thetoStringMapEntryListmethod for map serialization.
87-87: Map serialization approach changed for deterministic orderingThe code correctly implements the new approach to map serialization by converting the
extraDataMapto a list of string map entries usingProtoUtil.toStringMapEntryList(). This change aligns with the PR objective of ensuring deterministic serialization of maps in protobuf.core/src/main/java/bisq/core/payment/payload/MoneseAccountPayload.java (2)
20-21: Adding import for ProtoUtilThe import for
ProtoUtilis appropriately added to support the new map deserialization logic.
80-80: Changed map deserialization for deterministic orderingThe implementation correctly uses
ProtoUtil.toStringMap(proto.getExcludeFromJsonDataList())to convert the list of entries from protobuf to a map, matching the serialization changes for deterministic ordering as described in the PR objectives.core/src/main/java/bisq/core/payment/payload/SameBankAccountPayload.java (2)
20-21: Adding import for ProtoUtilThe import for
ProtoUtilis appropriately added to support the new map deserialization logic.
106-106: Changed map deserialization for deterministic orderingThe implementation correctly uses
ProtoUtil.toStringMap(proto.getExcludeFromJsonDataList())to convert the list of entries from protobuf to a map, supporting the PR's objective of ensuring deterministic serialization in protobuf.core/src/main/java/bisq/core/payment/payload/PaxumAccountPayload.java (2)
20-21: Adding import for ProtoUtilThe import for
ProtoUtilis appropriately added to support the new map deserialization logic.
79-79: Changed map deserialization for deterministic orderingThe implementation correctly uses
ProtoUtil.toStringMap(proto.getExcludeFromJsonDataList())to convert the list of entries from protobuf to a map, aligning with the PR's objective of ensuring deterministic serialization.core/src/main/java/bisq/core/payment/payload/PerfectMoneyAccountPayload.java (2)
20-21: Added ProtoUtil import for new serialization approachThis import is necessary to support the switch from map-based protobuf fields to repeated fields, which aligns with the PR's goal of ensuring deterministic serialization.
80-80: Updated deserialization to use list-based approachThe code now uses
ProtoUtil.toStringMap(proto.getExcludeFromJsonDataList())instead of directly accessing a map from the proto. This change is consistent with the PR objective to replace maps with repeated fields for deterministic serialization.core/src/main/java/bisq/core/payment/payload/CryptoCurrencyAccountPayload.java (2)
20-21: Added ProtoUtil import for new serialization approachThis import is necessary to support the switch from map-based protobuf fields to repeated fields, which aligns with the PR's goal of ensuring deterministic serialization.
74-74: Updated deserialization to use list-based approachThe code now uses
ProtoUtil.toStringMap(proto.getExcludeFromJsonDataList())instead of directly accessing a map from the proto. This change is consistent with the PR objective to replace maps with repeated fields for deterministic serialization.core/src/main/java/bisq/core/dao/state/model/governance/GenericProposal.java (3)
26-27: Added ProtoUtil import for new serialization approachThis import is necessary to support the switch from map-based protobuf fields to repeated fields, which aligns with the PR's goal of ensuring deterministic serialization.
38-38: Added empty line for code formattingThis change improves code readability by providing better visual separation between imports and class implementation.
87-87: Updated deserialization with null handling for empty collectionsThe code now properly handles empty collections by checking
CollectionUtils.isEmpty(proto.getExtraDataList())before attempting conversion. This is a good practice as it avoids creating unnecessary empty maps when no data is present.core/src/main/java/bisq/core/payment/payload/AdvancedCashAccountPayload.java (2)
20-21: Added ProtoUtil import for new serialization approachThis import is necessary to support the switch from map-based protobuf fields to repeated fields, which aligns with the PR's goal of ensuring deterministic serialization.
80-80: Updated deserialization to use list-based approachThe code now uses
ProtoUtil.toStringMap(proto.getExcludeFromJsonDataList())instead of directly accessing a map from the proto. This change is consistent with the PR objective to replace maps with repeated fields for deterministic serialization.core/src/main/java/bisq/core/dao/state/model/governance/ConfiscateBondProposal.java (2)
26-26: LGTM - Adding necessary import for ProtoUtil.The import for
ProtoUtilis required to support the new approach of converting list entries to a map.
95-95: Change from map to repeated field conversion looks good.This change aligns with the PR objective of replacing maps with repeated fields in protobuf for deterministic serialization. The code now correctly handles the list-based representation by converting it to a map using
ProtoUtil.toStringMap().core/src/main/java/bisq/core/payment/payload/InstantCryptoCurrencyPayload.java (2)
20-21: LGTM - Adding necessary import for ProtoUtil.The import for
ProtoUtilis required to support the new approach of converting list entries to a map.
74-74: Change from map to repeated field conversion looks good.The code now correctly uses
ProtoUtil.toStringMap()to convert the list-based representation from protobuf to a map. Thenew HashMap<>()construction ensures the returned map is mutable and independent of the original data.core/src/main/java/bisq/core/payment/payload/PopmoneyAccountPayload.java (2)
20-21: LGTM - Adding necessary import for ProtoUtil.The import for
ProtoUtilis required to support the new approach of converting list entries to a map.
85-85: Change from map to repeated field conversion looks good.The code now correctly uses
ProtoUtil.toStringMap()to convert the list-based representation from protobuf to a map. Thenew HashMap<>()construction ensures the returned map is mutable and independent of the original data.core/src/main/java/bisq/core/dao/state/model/governance/RoleProposal.java (2)
26-26: LGTM - Adding necessary import for ProtoUtil.The import for
ProtoUtilis required to support the new approach of converting list entries to a map.
105-105: Change from map to repeated field conversion looks good.This change aligns with the PR objective of replacing maps with repeated fields in protobuf for deterministic serialization. The code now correctly handles the list-based representation by converting it to a map using
ProtoUtil.toStringMap().core/src/main/java/bisq/core/payment/payload/SepaInstantAccountPayload.java (2)
20-20: Import added for ProtoUtil class to support the list-to-map conversion.This import is required for the new list-to-map conversion logic used in the
fromProtomethod.
123-123: Map serialization approach changed for deterministic ordering.The implementation now uses
ProtoUtil.toStringMapto convert a list of entries to a map, which ensures deterministic serialization as mentioned in the PR objectives. This change is part of the broader effort to replace maps with repeated fields in protobuf definitions.core/src/main/java/bisq/core/payment/payload/StrikeAccountPayload.java (2)
20-20: Import added for ProtoUtil class to support the list-to-map conversion.This import is required for the new list-to-map conversion logic used in the
fromProtomethod.
84-84: Map serialization approach changed for deterministic ordering.The implementation now uses
ProtoUtil.toStringMapto convert a list of entries to a map, which ensures deterministic serialization as mentioned in the PR objectives. This change is consistent with similar modifications made across other payload classes.core/src/main/java/bisq/core/payment/payload/UpholdAccountPayload.java (2)
20-20: Import added for ProtoUtil class to support the list-to-map conversion.This import is required for the new list-to-map conversion logic used in the
fromProtomethod.
90-90: Map serialization approach changed for deterministic ordering.The implementation now uses
ProtoUtil.toStringMapto convert a list of entries to a map, which ensures deterministic serialization as mentioned in the PR objectives. This change follows the consistent pattern applied across multiple payload classes.common/src/main/java/bisq/common/util/Utilities.java (1)
596-614: New utility method for converting snake_case to camelCase.This well-implemented utility method supports the normalization of field names across different programming languages as mentioned in the PR objectives. It properly handles edge cases (null/empty strings) and correctly converts snake_case to camelCase by capitalizing characters after underscores.
The implementation is clean and follows good practices:
- Checks for null/empty input first
- Uses a StringBuilder for efficient string construction
- Handles the conversion logic with a boolean flag to track when to capitalize
- Normalizes input to lowercase for consistency
core/src/main/java/bisq/core/payment/payload/SbpAccountPayload.java (2)
20-21: Import added for ProtoUtilProtoUtil is now imported to support the updated protobuf handling approach with repeated fields instead of maps.
92-92: Maps replaced with repeated field conversion for deterministic serializationThis change replaces direct map access with conversion from a repeated field list, aligning with the PR's goal of ensuring deterministic serialization. The
ProtoUtil.toStringMap()method converts the protobuf's repeated field list into a map structure.core/src/main/java/bisq/core/payment/payload/ImpsAccountPayload.java (2)
20-21: Import added for ProtoUtilProtoUtil is now imported to support the conversion between repeated fields and map structures.
92-92: Modified map handling for deterministic serializationChanged from directly using the protobuf map to using a list-based representation with
ProtoUtil.toStringMap(), ensuring consistent serialization order.core/src/main/java/bisq/core/payment/payload/UpiAccountPayload.java (2)
20-21: Import added for ProtoUtilAdded import for the utility class that handles conversion between repeated fields and maps.
84-84: Standardized approach to map serializationImplemented the consistent pattern of converting from repeated field lists to maps using
ProtoUtil.toStringMap(), which ensures deterministic serialization.core/src/main/java/bisq/core/payment/payload/CapitualAccountPayload.java (2)
20-21: Import added for ProtoUtilAdded import for the utility class needed for the new approach to handling map data.
80-80:✅ Verification successful
Consistent implementation of deterministic serialization pattern
Modified the map handling to use the list-based conversion approach via
ProtoUtil.toStringMap(). This change aligns with the PR objective of ensuring deterministic serialization of maps.
🏁 Script executed:
#!/bin/bash # Description: Verify that the ProtoUtil class contains the necessary methods used for conversion # Check if the ProtoUtil class has the toStringMap method rg -A 5 "public static Map<String, String> toStringMap" --type javaLength of output: 69
Below is an updated review comment that reflects our inability to automatically locate a definition for the conversion method. It now asks for a manual double-check to ensure the deterministic serialization implementation is complete:
Action: Verify the existence of the
ProtoUtil.toStringMap()method manuallyIt appears our initial automated search didn’t return any results for
toStringMap. Please confirm that theProtoUtilclass (typically in a file likeProtoUtil.java) includes the following conversion method:public static Map<String, String> toStringMap(/** parameters **/) { // method implementation… }If the method is missing or not implemented as expected, update the class accordingly to support the deterministic serialization pattern as intended by this PR. You may use the script below to further locate the file and occurrences of “toStringMap”:
Once confirmed, this will ensure map handling remains consistent with the PR objective.
🏁 Script executed:
#!/bin/bash # Description: Locate the ProtoUtil class and search for the toStringMap definition proto_util_file=$(fd -e java "ProtoUtil.java") if [ -z "$proto_util_file" ]; then echo "ProtoUtil.java not found; please verify the class location manually." else rg -n "toStringMap" "$proto_util_file" fiLength of output: 572
Action: Confirm deterministic serialization conversion
The verification confirms that the
ProtoUtilclass (located atcommon/src/main/java/bisq/common/proto/ProtoUtil.java) provides the list-based conversion approach via the method on line 130:public static LinkedHashMap<String, String> toStringMap(List<protobuf.StringMapEntry> entriesList)This method is correctly used in the payload conversion:
new HashMap<>(ProtoUtil.toStringMap(proto.getExcludeFromJsonDataList()))as it aligns with the PR objective to achieve deterministic serialization of maps. Please verify that wrapping the returned
LinkedHashMapin a newHashMapis intentional and does not inadvertently affect ordering where it is required.core/src/main/java/bisq/core/payment/payload/VerseAccountPayload.java (2)
20-20: Appropriate import addition for ProtoUtilThe import of
ProtoUtilis necessary to support the changes to the deserialization mechanism.
74-74: Implementation aligned with deterministic serialization goalThis change correctly implements the PR's objective by converting from the new repeated fields format (
getExcludeFromJsonDataList()) back to a map using theProtoUtil.toStringMap()utility method. This approach ensures deterministic serialization while maintaining compatibility with the existing code that expects a map.core/src/main/java/bisq/core/payment/payload/TransferwiseAccountPayload.java (2)
20-20: Appropriate import addition for ProtoUtilThe import of
ProtoUtilis necessary to support the changes to the deserialization mechanism.
81-81: Implementation aligned with deterministic serialization goalThis change correctly implements the PR's objective by converting from the new repeated fields format (
getExcludeFromJsonDataList()) back to a map using theProtoUtil.toStringMap()utility method. This approach ensures deterministic serialization while maintaining compatibility with the existing code that expects a map.core/src/main/java/bisq/core/payment/payload/PayseraAccountPayload.java (2)
20-20: Appropriate import addition for ProtoUtilThe import of
ProtoUtilis necessary to support the changes to the deserialization mechanism.
79-79: Implementation aligned with deterministic serialization goalThis change correctly implements the PR's objective by converting from the new repeated fields format (
getExcludeFromJsonDataList()) back to a map using theProtoUtil.toStringMap()utility method. This approach ensures deterministic serialization while maintaining compatibility with the existing code that expects a map.core/src/main/java/bisq/core/payment/payload/NeftAccountPayload.java (2)
20-20: Appropriate import addition for ProtoUtilThe import of
ProtoUtilis necessary to support the changes to the deserialization mechanism.
92-92: Implementation aligned with deterministic serialization goalThis change correctly implements the PR's objective by converting from the new repeated fields format (
getExcludeFromJsonDataList()) back to a map using theProtoUtil.toStringMap()utility method. This approach ensures deterministic serialization while maintaining compatibility with the existing code that expects a map.core/src/main/java/bisq/core/payment/payload/WeChatPayAccountPayload.java (1)
20-21: Implementation correctly adopts deterministic serializationThe addition of the ProtoUtil import and the change to use
ProtoUtil.toStringMap(proto.getExcludeFromJsonDataList())aligns with the goal of ensuring deterministic serialization. This approach replaces direct map access with a list-based representation that preserves entry order.Also applies to: 77-77
core/src/main/java/bisq/core/payment/payload/MoneyBeamAccountPayload.java (1)
20-21: Implementation correctly adopts deterministic serializationThe modification to use
ProtoUtil.toStringMap(proto.getExcludeFromJsonDataList())is consistent with the project-wide change to use repeated fields instead of maps for deterministic serialization. This approach ensures byte-level consistency in the serialized data.Also applies to: 82-82
core/src/main/java/bisq/core/payment/payload/BizumAccountPayload.java (1)
20-21: Implementation correctly adopts deterministic serializationThe change to use
ProtoUtil.toStringMap(proto.getExcludeFromJsonDataList())follows the consistent pattern of replacing map fields with repeated fields for deterministic serialization. This implementation helps ensure that serialized data maintains the same byte ordering across different instances.Also applies to: 84-84
core/src/main/java/bisq/core/payment/payload/CelPayAccountPayload.java (1)
20-21:❓ Verification inconclusive
Implementation correctly adopts deterministic serialization
The implementation properly follows the project-wide change to use list-based data structures instead of direct maps in protobuf. This consistent approach with
ProtoUtil.toStringMap(proto.getExcludeFromJsonDataList())ensures deterministic serialization, addressing the issue mentioned in the PR objectives.To verify backwards compatibility handling, consider running:
Also applies to: 74-74
🏁 Script executed:
#!/bin/bash # Find other occurrences of ProtoUtil.toStringMap() to ensure consistent implementation rg "ProtoUtil\.toStringMap" --type java # Look for usage of getExcludeFromJsonDataMap or similar map getters that might need updating rg "getExcludeFromJsonDataMap" --type java # Check if there are any remaining direct map field definitions in protobuf files rg "map<" --type protoLength of output: 17745
Verification Summary: Deterministic Serialization Consistency
The implementation in
core/src/main/java/bisq/core/payment/payload/CelPayAccountPayload.javacorrectly adopts deterministic serialization by using the list-to-map conversion withProtoUtil.toStringMap. Our shell script outputs show consistent usage across the codebase, with multiple similar calls found in other payment payloads and no instances of deprecated getters (e.g.,getExcludeFromJsonDataMap).
- Confirmed that calls to
ProtoUtil.toStringMapare used uniformly (e.g., inCelPayAccountPayload.javaand related payload classes).- No occurrences of the old map getter (
getExcludeFromJsonDataMap) were found.- Note: The check for direct map field definitions in protobuf files was inconclusive due to file type handling issues. Please manually verify that no direct map field definitions remain in the
.protofiles.Also applies to: lines 74-74.
core/src/main/java/bisq/core/payment/payload/NequiAccountPayload.java (2)
20-21: Added ProtoUtil import for map-to-list conversionThe import of ProtoUtil enables access to utility methods for handling the conversion between protocol buffer list representations and Java maps, which is consistent with the PR objective of using repeated fields for deterministic serialization.
84-84: Switched from direct map access to list-based representationThis change implements the PR's core objective by replacing direct map access (
getExcludeFromJsonDataMap()) with a list-based approach usingProtoUtil.toStringMap(proto.getExcludeFromJsonDataList()). This ensures deterministic serialization of maps in protobuf, addressing potential inconsistencies in map ordering during serialization.core/src/main/java/bisq/core/payment/payload/USPostalMoneyOrderAccountPayload.java (2)
20-21: Added ProtoUtil import for map-to-list conversionThe import enables access to utility methods for handling the conversion between protocol buffer list representations and Java maps, which aligns with the PR's goal of using repeated fields for deterministic serialization.
85-85: Switched from direct map access to list-based representationThis change implements the PR's objective by replacing direct map access (
getExcludeFromJsonDataMap()) with a list-based approach usingProtoUtil.toStringMap(proto.getExcludeFromJsonDataList()). This ensures deterministic serialization of maps in protocol buffers.core/src/main/java/bisq/core/payment/payload/RtgsAccountPayload.java (2)
20-21: Added ProtoUtil import for map-to-list conversionThe import provides access to utility methods for handling the conversion between protocol buffer list representations and Java maps, which is necessary for the changes in the fromProto method.
92-92: Switched from direct map access to list-based representationChanged from using direct map access to a list-based approach through
ProtoUtil.toStringMap(proto.getExcludeFromJsonDataList()). This change ensures deterministic serialization of maps in protocol buffers, which addresses the core issue this PR aims to solve.core/src/main/java/bisq/core/payment/payload/FasterPaymentsAccountPayload.java (2)
20-21: Added ProtoUtil import for map-to-list conversionThe import provides access to utility methods for handling the conversion between protocol buffer list representations and Java maps, consistent with changes across the codebase.
94-94:✅ Verification successful
Switched from direct map access to list-based representation
Changed from using direct map access to a list-based approach with
ProtoUtil.toStringMap(proto.getExcludeFromJsonDataList()). This ensures deterministic serialization in protocol buffers, addressing potential inconsistencies in map ordering during serialization.
🏁 Script executed:
#!/bin/bash # Verify that ProtoUtil.toStringMap is correctly handling the conversion from list to map # Find the ProtoUtil class to check its implementation fd -t f ProtoUtil.javaLength of output: 78
Deterministic serialization confirmed – no further changes required.
The verification confirms that the
ProtoUtil.javafile exists incommon/src/main/java/bisq/common/proto/ProtoUtil.javaand that itstoStringMapmethod is correctly employed to convert a list to a map. This implementation supports deterministic serialization as intended, mitigating issues with map ordering in protocol buffers.
- Verified file location:
common/src/main/java/bisq/common/proto/ProtoUtil.java- Confirmed usage in
FasterPaymentsAccountPayload.javawithnew HashMap<>(ProtoUtil.toStringMap(proto.getExcludeFromJsonDataList())))core/src/main/java/bisq/core/payment/payload/PaytmAccountPayload.java (2)
20-21: Import added for ProtoUtil.This new import supports the migration from map-based to list-based protobuf fields, which is part of the effort to ensure deterministic serialization.
84-84:✅ Verification successful
Changed from map to list-based deserialization.
The code now converts from a repeated list of StringMapEntry objects to a HashMap using ProtoUtil.toStringMap, replacing the direct access to getExcludeFromJsonDataMap(). This change aligns with the PR objective of using repeated fields for deterministic serialization.
🏁 Script executed:
#!/bin/bash # Let's verify that the ProtoUtil.toStringMap method exists and handles the conversion properly # Check the ProtoUtil class implementation fd --type file "ProtoUtil.java" | xargs catLength of output: 5449
Verified list-based deserialization conversion.
The change inPaytmAccountPayload.java(line 84) to convert from a repeated list ofStringMapEntryobjects usingProtoUtil.toStringMap(proto.getExcludeFromJsonDataList())is correct. The verification confirmed that theProtoUtil.toStringMapmethod exists and properly returns aLinkedHashMapconstructed from the repeated field, aligning with the PR objective for deterministic serialization.core/src/main/java/bisq/core/payment/payload/TikkieAccountPayload.java (2)
20-21: Import added for ProtoUtil.This new import is required for the conversion from list-based protobuf fields to maps.
84-84: Switched to deterministic serialization format.Changed from directly accessing the map to converting from a list representation, which ensures deterministic serialization. This is consistent with the changes in other payload classes.
core/src/main/java/bisq/core/payment/payload/PixAccountPayload.java (2)
20-21: Import added for ProtoUtil.This new import supports the conversion mechanism from protobuf repeated fields to Java maps.
86-86: Implemented deterministic serialization.Changed from map-based to list-based representation for excludeFromJsonData, ensuring deterministic serialization as specified in the PR objectives.
core/src/main/java/bisq/core/payment/payload/AchTransferAccountPayload.java (2)
20-21: Import added for ProtoUtil.Added import for the utility class that handles conversion between protobuf repeated fields and Java maps.
110-110:✅ Verification successful
Switched to list-based serialization format.
Changed from using the direct map access to converting from a list of entries, ensuring deterministic serialization across all implementations. The change is consistent with the modifications in other payment payload classes.
🏁 Script executed:
#!/bin/bash # Let's check if there are any places in the codebase where the old map method is still being used, # which could cause compatibility issues # Search for any remaining usages of getExcludeFromJsonDataMap rg "getExcludeFromJsonDataMap" --type java # Also check the protobuf definition to understand the changes made fd --type file "pb.proto" | xargs grep -A 10 "message StringMapEntry"Length of output: 434
Validation: List-based Serialization Implementation Confirmed
We verified that all references to the oldgetExcludeFromJsonDataMaphave been removed from the codebase. The updated code now correctly uses the list-basedproto.getExcludeFromJsonDataList(), ensuring deterministic serialization consistent with other payment payload classes. The protobuf definition forStringMapEntryaligns with this new approach.
- No use of the legacy map-based method was found in the codebase.
- The changes in
AchTransferAccountPayload.javaand corresponding protobuf updates are consistent and correctly implemented.core/src/main/java/bisq/core/payment/payload/TransferwiseUsdAccountPayload.java (2)
20-21: Added required utility for protobuf conversion.The import for ProtoUtil is necessary to handle the conversion from protobuf repeated fields to a string map.
94-94: Correctly implemented deterministic serialization pattern.This change replaces direct map access with a list-based approach, ensuring deterministic serialization of maps as mentioned in the PR objectives. The new HashMap wrapper ensures proper immutability.
core/src/main/java/bisq/core/dao/state/model/governance/ReimbursementProposal.java (2)
27-27: Added required utility for protobuf conversion.The import for ProtoUtil is necessary to support the conversion from protobuf repeated fields to a string map.
106-106: Correctly implemented deterministic serialization with null handling.This change replaces direct map access with a list-based approach, ensuring deterministic serialization of maps. The implementation properly handles null/empty cases using CollectionUtils.isEmpty check.
core/src/main/java/bisq/core/payment/payload/InteracETransferAccountPayload.java (2)
20-21: Added required utility for protobuf conversion.The import for ProtoUtil is necessary to support the conversion from protobuf repeated fields to a string map.
96-96: Correctly implemented deterministic serialization pattern.This change replaces direct map access with a list-based approach, consistent with the PR objectives to ensure deterministic serialization of maps. The new HashMap wrapper ensures proper immutability.
core/src/main/java/bisq/core/payment/payload/SwishAccountPayload.java (2)
20-21: Added required utility for protobuf conversion.The import for ProtoUtil is necessary to support the conversion from protobuf repeated fields to a string map.
83-83: Correctly implemented deterministic serialization pattern.This change replaces direct map access with a list-based approach, consistent with the PR objectives to ensure deterministic serialization of maps. The new HashMap wrapper ensures proper immutability.
core/src/main/java/bisq/core/payment/payload/OKPayAccountPayload.java (2)
20-21: Added import supports new protobuf serialization approachThe addition of the
ProtoUtilimport supports the transition from map-based to list-based protobuf representation for better deterministic serialization.
82-82: Implementation updated to use the new list-based protobuf structureThis change correctly implements the move from direct map access to using a list-based representation with conversion. The
ProtoUtil.toStringMap()method converts the protobuf list representation back to a HashMap as required by the internal data model.This approach ensures deterministic serialization of maps in the protobuf format while maintaining the existing Java API.
core/src/main/java/bisq/core/payment/payload/RevolutAccountPayload.java (3)
20-21: Added import supports new protobuf serialization approachThe addition of the
ProtoUtilimport supports the transition from map-based to list-based protobuf representation.
24-24: Empty line added for improved readabilityThe additional empty line adds clear separation between imports from different packages.
106-106: Implementation updated to use the new list-based protobuf structureThis change correctly implements the transition from direct map access to the new list-based representation with conversion. The
ProtoUtil.toStringMap()method converts the deterministic protobuf list into a HashMap as required by the internal data model.This implementation maintains backward compatibility for the internal Java API while ensuring deterministic serialization in the protobuf format.
core/src/main/java/bisq/core/payment/payload/ChaseQuickPayAccountPayload.java (2)
20-21: Added import supports new protobuf serialization approachThe addition of the
ProtoUtilimport supports the transition from map-based to list-based protobuf representation for more deterministic serialization.
88-88: Implementation updated to use the new list-based protobuf structureThis change correctly implements the transition from direct map access to using a list-based representation with conversion. The
ProtoUtil.toStringMap()method converts the protobuf list representation back to a HashMap as required by the internal data model.This approach ensures deterministic serialization of maps in protobuf while maintaining compatibility with the existing Java API.
common/src/main/java/bisq/common/ExcludeForHashAwareProto.java (2)
36-37: Added import for snake_case to camelCase conversionThis import correctly brings in the utility function needed for field name format conversion.
110-115: Improved field name matching to handle both snake_case and camelCase formatsThis enhancement correctly handles the field name matching between Java (typically camelCase) and protobuf (typically snake_case) by checking both formats. This is particularly important for the field name normalization mentioned in the PR description.
The logical restructuring with proper indentation and the OR condition improves readability while adding support for the enhanced matching logic.
core/src/main/java/bisq/core/payment/payload/SwiftAccountPayload.java (2)
20-21: New import for ProtoUtil class.Added import for
bisq.common.proto.ProtoUtilwhich is now used in thefromProtomethod to convert from repeated protobuf fields to a String map.
165-165:✅ Verification successful
Refactored to use deterministic serialization for excluded JSON data.
Modified the code to use
ProtoUtil.toStringMap(proto.getExcludeFromJsonDataList())instead of directly usingproto.getExcludeFromJsonDataMap(). This change ensures deterministic serialization of maps by using repeated fields in protobuf, which is the main goal of this PR.
🏁 Script executed:
#!/bin/bash # Verify that ProtoUtil.toStringMap is handling the conversion correctly rg -A 2 "toStringMap" --glob "*.java"Length of output: 35633
Deterministic Serialization for Excluded JSON Data Confirmed
The refactor now converts the excluded JSON data using
new HashMap<>(ProtoUtil.toStringMap(proto.getExcludeFromJsonDataList()))which ensures deterministic serialization by relying on repeated fields in protobuf. Our verification confirms that this pattern is applied consistently (e.g., in SwiftAccountPayload.java and multiple other payloads), matching the intended design. No further changes are needed.
core/src/main/java/bisq/core/dao/state/model/governance/CompensationProposal.java (2)
27-28: New import for ProtoUtil class.Added import for
bisq.common.proto.ProtoUtilwhich is now used to convert protobuf lists to maps.
111-111: Changed map to repeated fields for deterministic serialization.Modified the extraData handling to use
proto.getExtraDataList()with a null check viaCollectionUtils.isEmpty(), and then converting to a map usingProtoUtil.toStringMap(). This approach ensures deterministic serialization by replacing maps with repeated fields in protobuf.core/src/main/java/bisq/core/dao/state/model/governance/Cycle.java (1)
73-73:✅ Verification successful
Field name correction for consistency.
Changed from
proto.getHeightOfFirstLock()toproto.getHeightOfFirstBlock()to align with the protobuf field name change, ensuring proper deserialization.
🏁 Script executed:
#!/bin/bash # Verify that the protobuf definitions have been updated to match this change rg "height_of_first_block|height_of_first_lock" --glob "*.proto"Length of output: 203
Field Name Correction Verified
The change inCycle.javato callproto.getHeightOfFirstBlock()has been validated against the updated protobuf definitions. The search confirms thatproto/src/main/proto/pb.protonow correctly defines the field asheight_of_first_block, while the outdated definition remains inpb_old.proto. No further corrections are required.core/src/main/java/bisq/core/payment/payload/HalCashAccountPayload.java (2)
20-21: New import for ProtoUtil class.Added import for
bisq.common.proto.ProtoUtilwhich is now used in thefromProtomethod to convert from repeated protobuf fields to a String map.
78-78: Refactored to use deterministic serialization for excluded JSON data.Changed from directly using
proto.getExcludeFromJsonDataMap()toProtoUtil.toStringMap(proto.getExcludeFromJsonDataList()). This modification ensures deterministic serialization of maps by using repeated fields in protobuf instead of map fields, which is the main objective of this PR.core/src/main/java/bisq/core/payment/payload/NationalBankAccountPayload.java (2)
20-21: Added import for ProtoUtil to support the updated map handling.The
ProtoUtilimport is appropriately added to support the new way of handling map data in thefromProtomethod.
106-106: Map deserialization approach updated for deterministic serialization.The change from directly using
proto.getExcludeFromJsonDataMap()toProtoUtil.toStringMap(proto.getExcludeFromJsonDataList())aligns with the PR objective of ensuring deterministic serialization of maps by using repeated fields in protobuf.core/src/main/java/bisq/core/payment/payload/CashAppAccountPayload.java (2)
20-21: Added import for ProtoUtil to support the updated map handling.The
ProtoUtilimport is properly added to support the new way of handling map data in thefromProtomethod.
83-83: Updated map deserialization for deterministic handling.Changed from direct map usage to list-based approach with
ProtoUtil.toStringMap(proto.getExcludeFromJsonDataList())to ensure deterministic serialization.core/src/main/java/bisq/core/payment/payload/DomesticWireTransferAccountPayload.java (2)
20-21: Added import for ProtoUtil to support the updated map handling.The
ProtoUtilimport is correctly added to support the new approach for converting lists to maps in thefromProtomethod.
109-109: Map deserialization approach updated for deterministic serialization.Changed from direct map usage to list-based approach with
ProtoUtil.toStringMap(proto.getExcludeFromJsonDataList())to align with the PR's goal of ensuring deterministic serialization.core/src/main/java/bisq/core/user/PreferencesPayload.java (2)
193-193: Updated map serialization to use list-based approach.Changed from
putAllPeerTagMap(peerTagMap)toaddAllPeerTagMap(ProtoUtil.toStringMapEntryList(peerTagMap))to ensure deterministic serialization of maps in protobuf, which is a key objective of this PR.
294-294: Updated map deserialization to handle list-based data.Changed from
Maps.newHashMap(proto.getPeerTagMapMap())toMaps.newHashMap(ProtoUtil.toStringMap(proto.getPeerTagMapList()))to properly deserialize the new list-based representation of map data.p2p/src/main/java/bisq/network/p2p/storage/payload/MailboxStoragePayload.java (3)
24-24: New imports for protobuf conversion utilitiesThese imports support the refactoring from map-based to list-based protobuf representations, which enables deterministic serialization.
Also applies to: 28-28
122-122: Updated serialization approach for deterministic orderingThe code now uses the
ProtoUtil.toStringMapEntryList()method to convert a map to a list of key-value entries, instead of directly using map operations. This change ensures deterministic serialization order of map entries when the data is serialized to protobuf.
131-131: Updated deserialization approach to match new protobuf schemaThis line has been modified to use
getExtraDataList()instead ofgetExtraDataMap(), reflecting the protobuf schema change from maps to repeated fields. TheProtoUtil.toStringMap()utility converts the list back to a map for internal use.core/src/main/java/bisq/core/payment/payload/MoneyGramAccountPayload.java (2)
20-20: Added import for ProtoUtilThis import supports the conversion between list-based protobuf representations and map structures in Java.
100-100: Updated deserialization to use list-based protobuf representationChanged from directly accessing a map via
getExcludeFromJsonDataMap()to usinggetExcludeFromJsonDataList()with theProtoUtil.toStringMap()conversion utility. This change aligns with the PR objective of ensuring deterministic serialization by replacing maps with repeated fields in protobuf.core/src/main/java/bisq/core/payment/payload/SepaAccountPayload.java (2)
20-20: Added import for ProtoUtilThis import supports the conversion between list-based protobuf representations and map structures in Java.
128-128: Updated deserialization to use list-based protobuf representationChanged from directly accessing a map via
getExcludeFromJsonDataMap()to usinggetExcludeFromJsonDataList()with theProtoUtil.toStringMap()conversion utility. This maintains consistent behavior with the new protobuf schema that uses repeated fields instead of maps for deterministic serialization.core/src/main/java/bisq/core/payment/payload/MercadoPagoAccountPayload.java (2)
20-20: Added import for ProtoUtilThis import supports the conversion between list-based protobuf representations and map structures in Java.
86-86: Updated deserialization to use list-based protobuf representationChanged from directly accessing a map via
getExcludeFromJsonDataMap()to usinggetExcludeFromJsonDataList()with theProtoUtil.toStringMap()conversion utility. This change is consistent with updates in other payload classes and aligns with the PR objective of ensuring deterministic serialization.core/src/main/java/bisq/core/payment/payload/SatispayAccountPayload.java (2)
20-21: Import added for ProtoUtil.The new import is necessary to support the change from direct map access to the utility-based conversion method.
89-89: Changed from map to repeated field for deterministic serialization.Changed from direct map access (
proto.getExcludeFromJsonDataMap()) to using a list-based approach withProtoUtil.toStringMap(proto.getExcludeFromJsonDataList()). This change aligns with the PR objective to ensure deterministic serialization of maps in protobuf.core/src/main/java/bisq/core/network/p2p/inventory/messages/GetInventoryResponse.java (3)
23-23: Import added for ProtoUtil.The new import is necessary to support the conversion methods between maps and list-based representations.
63-63: Changed serialization from map to repeated field.Modified the serialization process to use
ProtoUtil.toStringMapEntryList(map)instead of direct map insertion. This change ensures deterministic serialization of maps in protobuf by using repeated fields.
69-69: Changed deserialization from map to repeated field.Updated the deserialization process to use
ProtoUtil.toStringMap(proto.getInventoryList())instead of directly accessing the map. This change complements the serialization change and maintains consistency in the data conversion approach.core/src/main/java/bisq/core/payment/payload/VenmoAccountPayload.java (2)
20-21: Import added for ProtoUtil.The new import is necessary to support the change from direct map access to the utility-based conversion method.
88-88: Changed from map to repeated field for deterministic serialization.Changed from direct map access (
proto.getExcludeFromJsonDataMap()) to using a list-based approach withProtoUtil.toStringMap(proto.getExcludeFromJsonDataList()). This change aligns with the PR objective to ensure deterministic serialization of maps in protobuf.Note that this change is particularly important for consistency even though this class is marked as @deprecated.
core/src/main/java/bisq/core/payment/payload/PromptPayAccountPayload.java (2)
20-21: Import added for ProtoUtil.The new import is necessary to support the change from direct map access to the utility-based conversion method.
79-79: Changed from map to repeated field for deterministic serialization.Changed from direct map access (
proto.getExcludeFromJsonDataMap()) to using a list-based approach withProtoUtil.toStringMap(proto.getExcludeFromJsonDataList()). This change aligns with the PR objective to ensure deterministic serialization of maps in protobuf.core/src/main/java/bisq/core/payment/payload/PaymentAccountPayload.java (3)
20-21: LGTM: New import for ProtoUtil utility class.The added import statement for ProtoUtil aligns with the PR goal of changing protobuf serialization.
39-39: LGTM: Import of Optional class.Adding this import supports potential null-handling operations in the code.
115-115: Serialization change to ensure deterministic output.The modification replaces a map-based approach (
putAllExcludeFromJsonData) with a list-based approach usingProtoUtil.toStringMapEntryList(). This change supports the PR's goal of ensuring deterministic serialization of maps in protobuf.core/src/main/java/bisq/core/payment/payload/JapanBankAccountPayload.java (2)
20-21: LGTM: New import for ProtoUtil utility class.The import is necessary for using the conversion methods in this class.
115-115: Updated deserialization to support deterministic protobuf format.Changed from directly using
proto.getExcludeFromJsonDataMap()to converting a list to a map withProtoUtil.toStringMap(proto.getExcludeFromJsonDataList()). This change aligns with the PR's objective of using repeated fields instead of maps for deterministic serialization.core/src/main/java/bisq/core/payment/payload/CashDepositAccountPayload.java (2)
20-21: LGTM: New import for ProtoUtil utility class.The import is necessary for the conversion utilities used in the fromProto method.
133-133: Updated deserialization to support deterministic protobuf format.Changed from directly using
proto.getExcludeFromJsonDataMap()to converting a list to a map withProtoUtil.toStringMap(proto.getExcludeFromJsonDataList()). This modification ensures compatibility with the new deterministic serialization format outlined in the PR objectives.core/src/main/java/bisq/core/payment/payload/SpecificBanksAccountPayload.java (2)
20-21: LGTM: New import for ProtoUtil utility class.The import is required for the new conversion utility used in the fromProto method.
121-121: Updated deserialization to support deterministic protobuf format.Changed from directly using
proto.getExcludeFromJsonDataMap()to converting a list to a map withProtoUtil.toStringMap(proto.getExcludeFromJsonDataList()). This change is consistent with other similar modifications in the PR and ensures deterministic serialization of map data.core/src/main/java/bisq/core/alert/Alert.java (3)
27-27: Added import for ProtoUtil.This import is required for the conversion between string maps and protobuf StringMapEntry lists in the modified methods below.
119-119: Replaced map with deterministic list representation for serialization.This change converts the extraDataMap to a list of StringMapEntry objects using
ProtoUtil.toStringMapEntryList()instead of directly adding the map to the protobuf message. This follows the PR objective of ensuring deterministic serialization of maps.
136-136: Updated deserialization to handle list-based representation.Changed from checking
proto.getExtraDataMap()to checkingproto.getExtraDataList()and usingProtoUtil.toStringMap()for conversion, maintaining consistency with the serialization changes.core/src/main/java/bisq/core/support/dispute/arbitration/arbitrator/Arbitrator.java (2)
93-93: Replaced map with deterministic list representation for serialization.Changed from using
putAllExtraDatato usingaddAllExtraDatawithProtoUtil.toStringMapEntryList(map), ensuring deterministic serialization of maps consistent with the PR objectives.
108-108: Updated deserialization to handle list-based representation.Modified to check
proto.getExtraDataList()instead ofproto.getExtraDataMap()and useProtoUtil.toStringMap()for conversion, maintaining consistency with the serialization changes.core/src/main/java/bisq/core/offer/bisq_v1/OfferPayload.java (2)
278-278: Replaced map with deterministic list representation for serialization.Modified to use
addAllExtraDatawithProtoUtil.toStringMapEntryList(map)instead of directly adding the map to the protobuf message, ensuring deterministic serialization.
290-291: Updated deserialization to handle list-based representation.Changed from using
getExtraDataMap()to usinggetExtraDataList()withProtoUtil.toStringMap()for conversion back to a map, maintaining consistency with the serialization changes.core/src/main/java/bisq/core/dao/state/model/governance/ChangeParamProposal.java (2)
26-26: Added import for ProtoUtil.This import is necessary for the conversion from protobuf list to map in the fromProto method.
102-102: Updated deserialization to handle list-based representation.Changed from checking
proto.getExtraDataMap()to checkingproto.getExtraDataList()and usingProtoUtil.toStringMap()for conversion, maintaining consistency with the serialization pattern used across the codebase.core/src/main/java/bisq/core/support/dispute/refund/refundagent/RefundAgent.java (2)
86-86: Improved serialization determinismChanged from using map-based
putAllExtraDatato using list-basedaddAllExtraDatawithProtoUtil.toStringMapEntryList. This ensures deterministic serialization of maps in protobuf, addressing the core issue mentioned in the PR objectives.
99-99: Updated deserialization to match new protobuf schemaChanged from checking
proto.getExtraDataMap()to checkingproto.getExtraDataList()and usingProtoUtil.toStringMapto convert the list back to a map. This complements the serialization change and maintains consistency.core/src/main/java/bisq/core/dao/governance/proposal/storage/temp/TempProposalPayload.java (3)
27-27: Imported ProtoUtil for conversion utilitiesAdded import for the ProtoUtil class which contains the methods needed for map/list conversions.
100-100: Improved serialization determinismChanged from using map-based
putAllExtraDatato using list-basedaddAllExtraDatawithProtoUtil.toStringMapEntryList. This ensures deterministic serialization of maps in protobuf, consistent with changes across the codebase.
112-112: Updated deserialization to match new protobuf schemaChanged from checking
proto.getExtraDataMap()to checkingproto.getExtraDataList()and usingProtoUtil.toStringMapto convert the list back to a map. This complements the serialization change and maintains consistency.core/src/main/java/bisq/core/user/UserPayload.java (3)
30-31: Updated imports for the ProtoUtil classReorganized imports to include Proto and ProtoUtil for the conversion utilities.
169-169: Improved cookie serialization determinismChanged from using map-based cookie data to list-based with
addAllCookieandProtoUtil.toStringMapEntryList. This ensures deterministic serialization of the cookie data, consistent with the approach used for other map fields.
218-218: Updated cookie deserialization to match new protobuf schemaChanged from using
proto.getCookieMap()to usingproto.getCookieList()andProtoUtil.toStringMapto convert back to a map. This complements the serialization change and ensures data consistency.core/src/main/java/bisq/core/support/dispute/mediation/mediator/Mediator.java (2)
80-80: Improved serialization determinismChanged from using map-based
putAllExtraDatato using list-basedaddAllExtraDatawithProtoUtil.toStringMapEntryList. This ensures deterministic serialization of maps in protobuf, consistent with similar changes across the codebase.
93-93: Updated deserialization to match new protobuf schemaChanged from checking
proto.getExtraDataMap()to checkingproto.getExtraDataList()and usingProtoUtil.toStringMapto convert the list back to a map. This complements the serialization change and maintains consistency.core/src/main/java/bisq/core/payment/payload/CashByMailAccountPayload.java (2)
20-21: Import added for ProtoUtilThe import of
ProtoUtilclass is required to support the new conversion from repeated field list to map.
90-90: Replaced map with repeated fields for deterministic serializationChanged from directly using
proto.getExcludeFromJsonDataMap()to converting a list to a map usingProtoUtil.toStringMap(proto.getExcludeFromJsonDataList()). This implements the PR objective of replacing maps with repeated fields for deterministic serialization.core/src/main/java/bisq/core/payment/payload/AliPayAccountPayload.java (2)
20-21: Import added for ProtoUtilThe import of
ProtoUtilclass is required to support the new conversion from repeated field list to map.
77-77: Replaced map with repeated fields for deterministic serializationChanged from directly using
proto.getExcludeFromJsonDataMap()to converting a list to a map usingProtoUtil.toStringMap(proto.getExcludeFromJsonDataList()). This change ensures consistent deterministic serialization across the codebase.core/src/main/java/bisq/core/payment/PaymentAccount.java (2)
111-111: Changed extraData serialization to use repeated fieldsModified the serialization of extraData to use
addAllExtraDatawithProtoUtil.toStringMapEntryList(map)instead ofputAllExtraData. This change converts the map to a list of entries, aligning with the goal of using repeated fields for deterministic serialization.
142-146: Updated extraData deserialization to handle repeated fieldsModified the deserialization logic to:
- Check for empty list instead of map presence with
CollectionUtils.isEmpty(proto.getExtraDataList())- Convert the list back to a map using
ProtoUtil.toStringMap(proto.getExtraDataList())This change complements the serialization changes and ensures bidirectional compatibility.
core/src/main/java/bisq/core/payment/payload/F2FAccountPayload.java (2)
20-21: Import added for ProtoUtilThe import of
ProtoUtilclass is required to support the new conversion from repeated field list to map.
100-100: Replaced map with repeated fields for deterministic serializationChanged from directly using
proto.getExcludeFromJsonDataMap()to converting a list to a map usingProtoUtil.toStringMap(proto.getExcludeFromJsonDataList()). This maintains consistency with the other similar changes throughout the codebase.core/src/main/java/bisq/core/filter/Filter.java (2)
465-465: Update for deterministic serialization of mapsThe change from
putAllExtraDatatoaddAllExtraData(ProtoUtil.toStringMapEntryList(map))supports deterministic serialization of map data. Protocol Buffers don't guarantee deterministic ordering when serializing maps directly, but using repeated fields with key-value pairs ensures consistent serialization order.
497-497: Consistent deserialization approach for extraDataMapThe corresponding change in the
fromProtomethod properly handles the new data structure by checkingproto.getExtraDataList()instead of the previous map implementation, and converts it back to a map usingProtoUtil.toStringMap().core/src/main/java/bisq/core/payment/payload/AustraliaPayidAccountPayload.java (2)
20-21: Added new import for ProtoUtilThe ProtoUtil import has been added to support the new map-to-list conversion functionality.
89-89: Updated deserialization for deterministic data handlingThe change from using
proto.getExcludeFromJsonDataMap()toProtoUtil.toStringMap(proto.getExcludeFromJsonDataList())aligns with the project's goal of ensuring deterministic serialization by using repeated fields instead of maps in protobuf definitions.core/src/main/java/bisq/core/payment/payload/AmazonGiftCardAccountPayload.java (2)
20-21: Added new import for ProtoUtilThe ProtoUtil import has been added to support the new map-to-list conversion functionality.
93-93: Updated deserialization for deterministic data handlingThe change to
ProtoUtil.toStringMap(proto.getExcludeFromJsonDataList())correctly implements the new approach of using repeated fields for map data in protobuf. Unlike other implementations, this one doesn't check for empty lists before conversion. This is fine if the field is always populated or if the conversion method handles empty lists gracefully.core/src/main/java/bisq/core/trade/statistics/TradeStatistics3.java (2)
338-338: Update for deterministic serialization of mapsThe change from
putAllExtraDatatoaddAllExtraData(ProtoUtil.toStringMapEntryList(map))ensures deterministic serialization of map data by using repeated field of key-value entries instead of direct map serialization.
360-360: Consistent deserialization approach for extraDataMapThe corresponding change in the
fromProtomethod properly handles the new data structure by checkingproto.getExtraDataList()instead of the previous map implementation, and converts it back to a map usingProtoUtil.toStringMap().core/src/main/java/bisq/core/offer/bsq_swap/BsqSwapOfferPayload.java (2)
131-131: Changed map to repeated field for extraData serializationThis change replaces the use of map-based
putAllExtraDatawith list-basedaddAllExtraDataand converts theextraDataMapto a list of string entries. This aligns with the PR objective of ensuring deterministic serialization of maps in protobuf.
137-138: Updated deserialization to handle repeated fieldsThe deserialization now properly checks for an empty list of extra data entries instead of an empty map, and converts the list back to a map using
ProtoUtil.toStringMap(). This maintains consistency with the serialization change above.core/src/main/java/bisq/core/support/dispute/Dispute.java (2)
291-291: Replaced map serialization with repeated field patternModified serialization to use
addAllExtraDatawith a list conversion rather thanputAllExtraData, which is consistent with the project-wide change to ensure deterministic serialization of map data in protobuf.
317-318: Updated deserialization to support repeated field patternThe deserialization now properly checks for an empty list of extra data entries and converts the list back to a map. The change also maintains validation using
ExtraDataMapValidator, ensuring data integrity despite the serialization format change.core/src/main/java/bisq/core/dao/state/model/governance/RemoveAssetProposal.java (2)
26-26: Added ProtoUtil import for protobuf conversionsAdded import for the ProtoUtil class used in the updated fromProto method.
96-96: Updated deserialization to use list-based extraDataChanged the extraData deserialization to check for an empty list (
proto.getExtraDataList()) instead of an empty map, and convert the list to a map usingProtoUtil.toStringMap(). This maintains the expected functionality while adapting to the new protobuf schema.core/src/main/java/bisq/core/payment/payload/WesternUnionAccountPayload.java (2)
20-21: Added ProtoUtil import for protobuf conversionsAdded import for the ProtoUtil class used in the updated fromProto method.
107-107: Updated excludeFromJsonData deserialization to use repeated fieldsChanged the deserialization of excludeFromJsonData to use
ProtoUtil.toStringMap(proto.getExcludeFromJsonDataList())instead of accessing the map directly. This aligns with the PR goal of using repeated fields instead of maps in protobuf definitions for deterministic serialization.core/src/main/java/bisq/core/dao/state/model/DaoState.java (7)
35-37: New imports for map entry protobuf classes.These new imports support the transition from map fields to repeated fields in the protobuf definitions, aligning with the PR objective to ensure deterministic serialization.
200-210: Replaced map with repeated field pattern for unspentTxOutputMap.The code has been updated to convert the TreeMap to a list of BaseTxOutputMapEntry objects for the protobuf message, which ensures deterministic serialization. The multi-step transformation (converting to a string key map, then to entry objects, then collecting to a list) is a bit verbose but necessary given protobuf limitations.
211-220: Replaced map with repeated field pattern for spentInfoMap.Similar to the unspentTxOutputMap change, this code converts the spentInfoMap to a list of SpentInfoMapEntry objects for the protobuf message. The approach is consistent with the other map conversions in this file.
222-231: Replaced map with repeated field pattern for issuanceMap.The implementation follows the same pattern as the other map conversions, converting the issuanceMap to a list of IssuanceMapEntry objects. This consistent approach across all maps helps maintain code readability.
248-249: Updated deserialization to use the new list-based field.The code now properly deserializes from the new repeated field format (unspentTxOutputMapEntriesList) instead of the old map format, maintaining compatibility with the serialization changes.
250-251: Updated deserialization for spentInfoMap.Similar to the unspentTxOutputMap change, this code now reads from the list-based field rather than the map-based field.
253-254: Updated deserialization for issuanceMap.This completes the changes required for deserialization from the new protobuf format, ensuring that all map structures are properly read from their list-based representations.
core/src/main/java/bisq/core/dao/governance/blindvote/BlindVote.java (3)
22-22: Added ProtoUtil import for String map conversion.Added import for the utility class that handles conversion between maps and list-based protobuf string map entries.
101-101: Updated extraDataMap serialization to use repeated field pattern.Changed from
putAllExtraDatatoaddAllExtraDatawithProtoUtil.toStringMapEntryList()to align with the new protobuf definition that uses repeated fields instead of maps. This ensures deterministic serialization of the map entries.
111-111: Updated extraDataMap deserialization to use the list-based field.Changed the deserialization to use
getExtraDataList()withProtoUtil.toStringMap()instead ofgetExtraDataMap(), maintaining compatibility with the serialization changes.core/src/main/java/bisq/core/trade/statistics/TradeStatistics2.java (2)
228-228: Updated extraDataMap serialization to use repeated field pattern.Changed from
putAllExtraDatatoaddAllExtraDatawithProtoUtil.toStringMapEntryList()to align with the new protobuf definition that uses repeated fields instead of maps. This ensures deterministic serialization of the map entries.
259-259: Updated extraDataMap deserialization to use the list-based field.Changed the deserialization to use
getExtraDataList()withProtoUtil.toStringMap()instead ofgetExtraDataMap(), maintaining compatibility with the serialization changes.core/src/test/java/bisq/core/protobuf/SerializationTest.java (3)
21-63: Well-structured test for verifying protobuf serialization compatibility.This test verifies that the old and new protobuf definitions produce the same serialized bytes for single entries and empty values. It also tests that the old protobuf can correctly deserialize data serialized with the new protobuf format, which is crucial for backward compatibility.
The test covers important compatibility cases:
- Single entry serialization equivalence
- Empty string value handling
- Multiple entries deserialization
This helps ensure the changes won't break compatibility between clients using different protobuf versions.
65-116: Comprehensive DaoState serialization test using real data.This test reads serialized DaoState data from a file and verifies that both old and new protobuf definitions can parse it correctly. It also validates that the content is equivalent by checking that all entries from the old maps are present in the new lists.
The approach is thorough:
- Loading binary test data from resources
- Parsing with both old and new protobuf classes
- Comparing serialized sizes
- Verifying all map entries are present in the corresponding lists
This test is essential for validating that the DaoState serialization changes maintain backward compatibility with existing data.
118-135: Important test for map order preservation.This test addresses a key issue mentioned in the PR objectives - ensuring deterministic serialization of maps. It specifically tests problematic keys that caused ordering issues in the C implementation of protobuf, verifying that the order is preserved after multiple serialization/deserialization cycles.
The test:
- Creates a map with known problematic keys
- Performs 50 rounds of serialization/deserialization
- Verifies the byte representation remains unchanged
This test provides confidence that the serialization changes will address the deterministic ordering issues that motivated this PR.
proto/src/main/proto/pb.proto (4)
181-182: Review: GetInventoryResponse UpdateThe
inventoryfield inGetInventoryResponsehas been changed to a repeated field of typeStringMapEntry, ensuring deterministic serialization. Make sure all client code consuming this message is updated to parse the new repeated format.
636-642: Review: ProtectedStorageEntry UpdateThe definition of
ProtectedStorageEntrynow explicitly declares the fieldstorage_payload(line 637) with its type. No issues were found here; the update appears to be consistent with the new patterns.
2251-2264: Review: New Map Entry Message TypesThe new messages:
•BaseTxOutputMapEntry(lines 2251–2254)
•IssuanceMapEntry(lines 2256–2259)
•SpentInfoMapEntry(lines 2261–2264)replace the previous map structures. This change will help enforce deterministic ordering. Please double-check that all conversion utilities and serializer/deserializer functions have been updated to use these new definitions.
720-721: Review: Consistent Conversion of extra_data FieldsAcross multiple messages (e.g. in Arbitrator, Mediator, RefundAgent, Filter, TradeStatistics2, TradeStatistics3, MailboxStoragePayload, OfferPayload and within DaoState), the previously defined map fields for extra data have been replaced by a repeated
StringMapEntryfield. This is consistent with the overall objective of deterministic serialization. Ensure that all clients perform the necessary conversions during serialization/deserialization.Also applies to: 728-729, 740-741, 749-750, 802-803, 814-815, 821-822, 863-865, 2272-2273, 2275-2276
proto/src/main/proto/pb_old.proto (1)
1-16: Review: New Legacy Protobuf DefinitionThis new file defines many legacy (old-proto) message types under the package
io.bisq.protobuffer.old. Overall, the definitions match the expected design pattern and appear consistent with the rest of the codebase.
| @@ -59,7 +59,7 @@ public Cycle(int heightOfFirstBlock, ImmutableList<DaoPhase> daoPhaseList) { | |||
| @Override | |||
| public protobuf.Cycle toProtoMessage() { | |||
| return protobuf.Cycle.newBuilder() | |||
| .setHeightOfFirstLock(heightOfFirstBlock) | |||
| .setHeightOfFirstBlock(heightOfFirstBlock) | |||
There was a problem hiding this comment.
Field name correction for consistency.
Changed from setHeightOfFirstLock to setHeightOfFirstBlock to maintain consistency with the field name used in the class (see field declaration at line 42: private final int heightOfFirstBlock).
| @@ -0,0 +1,2595 @@ | |||
| /* | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Static Analysis: Directory Structure for pb_old.proto
The file’s package is defined as io.bisq.protobuffer.old yet it is located in the proto/src/main/proto/ directory. According to the static analysis hint, files with this package should reside under the directory io/bisq/protobuffer/old relative to the repository root. Please move pb_old.proto accordingly. For example, you could run:
git mv proto/src/main/proto/pb_old.proto io/bisq/protobuffer/old/pb_old.protoThis will help ensure that the package-to-directory structure is enforced throughout the project.
This change looks quite large but it's very simple: It changes maps to repeated fields for deterministic serialization of maps
This is as we discussed it in the matrix chat
I have left the DaoState to still use hashmap before serializing to the new deterministic approach as we need to discuss it further
the reason for having separate definition for each entry is because protobuf does not have generics and using oneof for that purpose in our case seemed like a bad choice
I have also added a test to demonstrate the change and how serialization of bytes are affected
the test also serves as a tool to detect if somehow protobuf decides to do things differently in future, so we don't fail silently in case serialization changes
but the test needs improvement
one immediate issue that I realized right now is that if newer client sends data back to an old client, and old client tries to parse that into a hashmap (using
Collectors.toMap), the order can/will be different, which can have complicationsthe other way around is not an issue though.
I also normalized a few protobuf field names to snake case so that they are not generated weirdly on python and possibly other languages, like the rest of the fields
Summary by CodeRabbit