refactor: AccountInterface: use photon v2 types and simplify ColdContext#2274
Conversation
📝 WalkthroughWalkthroughThis pull request refactors the cold (compressed) account representation by consolidating an enumerated Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes The changes involve heterogeneous structural modifications across multiple interconnected modules. Type signature changes in core public APIs (AccountInterface.cold, TokenAccountInterface.cold), removal of an entire enum pattern with cascading effects on accessor logic, introduction of new helper functions with non-trivial deserialization paths, and elimination of legacy conversion paths require careful validation that the new unified data flow maintains correctness without losing logic or introducing subtle bugs. Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
- Removed `make_get_token_account_interface_body` and `make_get_ata_interface_body` functions from the photon API module. - Updated various test files to remove references to `ColdContext` and directly use compressed accounts in `AccountInterface`. - Adjusted `AccountSpec::Ata` instantiation to wrap `ata_interface` in `Box::new`. - Cleaned up imports in multiple test files by removing unused `ColdContext` references.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
sdk-libs/client/src/indexer/types/interface.rs (1)
127-137:⚠️ Potential issue | 🟠 MajorGuard against multiple cold entries instead of silently picking the first.
Line 131 currently takesentries.first(). If the API ever returns more than one cold entry, the extras are ignored silently, which can lead to incorrect hashes/proofs. Consider rejecting multiple entries explicitly.🔧 Suggested fix
- let cold = ai - .cold - .as_ref() - .and_then(|entries| entries.first()) - .map(convert_account_v2) - .transpose()?; + let cold = match ai.cold.as_ref() { + Some(entries) if entries.len() > 1 => { + return Err(IndexerError::InvalidParameters( + "expected at most one cold entry".to_string(), + )); + } + Some(entries) => entries.first().map(convert_account_v2).transpose()?, + None => None, + };sdk-libs/client/src/indexer/photon_indexer.rs (1)
1764-1870:⚠️ Potential issue | 🟠 MajorAdd owner validation for hot token accounts before returning TokenData.
The
parse_token_data_from_indexer_accountfunction returns a defaultTokenDatafor on-chain accounts without checking that the account is owned bylight_token_interface::LIGHT_TOKEN_PROGRAM_ID. Sinceget_token_account_interfaceaccepts any address and routes throughget_account_interface, a non-token account could pass through with misleading default token data. The codebase already validates mint account ownership in a similar context (seeclient.rsmint path), establishing this as a required check.🔧 Suggested fix
fn parse_token_data_from_indexer_account( ai: &AccountInterface, ) -> Result<light_token::compat::TokenData, IndexerError> { match &ai.cold { Some(cold) => borsh::BorshDeserialize::deserialize(&mut cold.data.data.as_slice()) .map_err(|e| IndexerError::decode_error("token_data", e)), None => { + let expected = + Pubkey::new_from_array(light_token_interface::LIGHT_TOKEN_PROGRAM_ID); + if ai.account.owner != expected { + return Err(IndexerError::InvalidParameters(format!( + "expected token account owner {}, got {}", + expected, ai.account.owner + ))); + } // Hot account — downstream will re-parse from SPL account data directly Ok(light_token::compat::TokenData::default()) } } }
Summary by CodeRabbit
API Changes
get_token_account_interface_postendpointget_ata_interface_postendpointRefactor