Conversation
There was a problem hiding this comment.
Code review for Solana compare followup PR. I've identified several issues that warrant attention:
Logic Issues:
known_ui_amount_mismatch_in- The condition appears inverted; it returnstruewhen amounts are close (within tolerance) rather than when there's a mismatchknown_block_reward_count_mismatch- Returnstruewhen counts still don't match, but semantically should returntruewhen the mismatch is "explained" (i.e., counts match after accounting for different storage)- Transaction status skip - Skips comparison on any status difference, but the comment mentions a specific error type; consider being more precise
Data Integrity Concerns:
4. Zero ui_amount conversion - Converting 0.0 to None conflates "zero tokens" with "field not populated", which could cause data loss
Minor Items:
5. Retry configuration - Using default ExponentialBuilder settings; consider making explicit for clarity
6. Block reward comparison - When counts mismatch (even if "known"), the zip comparison will silently ignore extra rewards
7. Log level change - Channel close changed from debug! to error! - verify this is truly an error condition vs normal shutdown
The PR adds useful retry logic and handles several known OF1/RPC mismatches, but the logic in the mismatch detection functions should be verified.
d376de2 to
2142b7a
Compare
Fixes slight mismatches in the values of some fields between OF1 and JSON-RPC slots (using the JSON-RPC values as the source of truth): - If the OF1 pre/post balance or reward optionals are missing, turn them into empty vectors. - If the OF1 value/owner TransactionTokenBalance fields are empty strings, replace them with `None`. - If the OF1 ui_amount TokenBalance field is equal to 0.0, replace it with `None`.
There are a few currently known mismatches between the OF1 and JSON-RPC slot fields: - JSON-RPC slots have their `blocktime` fields populated very early on, while those same slots have `None` or `Some(0)` blocktime values in OF1 data. - Some OF1 transactions have an error set (always the same one, `InstructionError::Custom(0)`) whereas those transactions in JSON-RPC have no error set. - Block rewards are not populated the same way between OF1 and JSON-RPC. In the JSON-RPC response they are distributed across individual transactions' metadata fields. In the OF1 response they are stored in a top level `block_rewards field`. For now, the first two will be documented and set aside. The third one has to be investigated further since it can probably be resolved through JSON-RPC call configuration.
2142b7a to
969aa6f
Compare
There are a few currently known mismatches between the OF1 and JSON-RPC slot fields:
blocktimefields populated very early on, while those same slots haveNoneorSome(0)blocktime values in OF1 data.InstructionError::Custom(0)) whereas those transactions in JSON-RPC have no error set.block_rewards field.For now, the first two will be documented and set aside. The third one has to be investigated further since it can probably be resolved through JSON-RPC call configuration.