fix: use connected pda seed for execute and execute spl #3637
Conversation
skosito
added
no-changelog
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThis pull request updates the codebase to replace all calls to the deprecated Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Test/Runner
participant Contract as solanacontract.ComputeConnectedPdaAddress
participant Checker as require.NoError
Client->>Contract: ComputeConnectedPdaAddress(connected)
Contract-->>Client: PDA Address or error
Client->>Checker: Validate error status
Possibly related PRs
Suggested reviewers
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. 🪧 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
Documentation and Community
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #3637 +/- ##
========================================
Coverage 64.56% 64.56%
========================================
Files 469 469
Lines 32772 32772
========================================
Hits 21159 21159
Misses 10648 10648
Partials 965 965
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
zetaclient/chains/solana/signer/execute_spl.go (1)
50-54: Function replacement may need error message adjustment.The function call has been updated to use
ComputeConnectedPdaAddressinstead of the previously usedComputeConnectedSPLPdaAddress, which aligns with the systematic refactoring across the codebase. However, the error message on line 53 still references "cannot decode connected spl pda address" which is inconsistent with the generalized function now being used.Consider updating the error message to reflect the more general PDA computation:
- return nil, errors.Wrap(err, "cannot decode connected spl pda address") + return nil, errors.Wrap(err, "cannot compute connected pda address")Additionally, this code path lacks test coverage according to static analysis, which should be addressed to ensure the refactoring functions correctly.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 51-51: zetaclient/chains/solana/signer/execute_spl.go#L51
Added line #L51 was not covered by tests
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
contrib/localnet/solana/connected_spl.sois excluded by!**/*.so
📒 Files selected for processing (6)
e2e/e2etests/test_spl_withdraw_and_call.go(1 hunks)e2e/e2etests/test_spl_withdraw_and_call_revert.go(1 hunks)e2e/runner/setup_solana.go(1 hunks)e2e/runner/solana.go(1 hunks)pkg/contracts/solana/gateway.go(0 hunks)zetaclient/chains/solana/signer/execute_spl.go(2 hunks)
💤 Files with no reviewable changes (1)
- pkg/contracts/solana/gateway.go
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.go`: Review the Go code, point out issues relative to ...
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
e2e/e2etests/test_spl_withdraw_and_call_revert.goe2e/runner/solana.goe2e/e2etests/test_spl_withdraw_and_call.goe2e/runner/setup_solana.gozetaclient/chains/solana/signer/execute_spl.go
🪛 GitHub Check: codecov/patch
zetaclient/chains/solana/signer/execute_spl.go
[warning] 51-51: zetaclient/chains/solana/signer/execute_spl.go#L51
Added line #L51 was not covered by tests
[warning] 110-110: zetaclient/chains/solana/signer/execute_spl.go#L110
Added line #L110 was not covered by tests
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: start-solana-test / e2e
- GitHub Check: start-e2e-test / e2e
🔇 Additional comments (4)
e2e/e2etests/test_spl_withdraw_and_call_revert.go (1)
53-53: Updated function name to use generic PDA computationThe function call has been updated from
ComputeConnectedSPLPdaAddresstoComputeConnectedPdaAddress, which aligns with the standardization of PDA address computation across the codebase. This change provides consistency with the related Solana PR mentioned in the objectives.e2e/e2etests/test_spl_withdraw_and_call.go (1)
56-56: Updated function name to use generic PDA computationThe function call has been updated from
ComputeConnectedSPLPdaAddresstoComputeConnectedPdaAddress, which consolidates the PDA computation logic. This is consistent with the changes in other files and follows the refactoring approach outlined in the PR objectives.e2e/runner/solana.go (1)
578-578:✅ Verification successful
Updated function name to use generic PDA computation
The function call has been updated from
ComputeConnectedSPLPdaAddresstoComputeConnectedPdaAddress. This change maintains functional equivalence while providing a more consistent API for PDA address computation.
🏁 Script executed:
#!/bin/bash # Verify if there are any remaining instances of ComputeConnectedSPLPdaAddress in the codebase echo "Searching for any remaining references to ComputeConnectedSPLPdaAddress:" rg "ComputeConnectedSPLPdaAddress" --type go echo "Done."Length of output: 218
Update Verified: Consistent API Naming for PDA Computation
The change from
ComputeConnectedSPLPdaAddresstoComputeConnectedPdaAddresshas been verified. The shell script search confirmed that no references to the deprecated function remain in the codebase. The updated function call ate2e/runner/solana.goat line 578 now consistently uses the generic naming convention, ensuring a more uniform API.e2e/runner/setup_solana.go (1)
98-98:✅ Verification successful
Updated function name to use generic PDA computation
The function call has been updated from
ComputeConnectedSPLPdaAddresstoComputeConnectedPdaAddress. This change is part of the standardization of PDA address computation across the codebase, making it more maintainable and consistent.
🏁 Script executed:
#!/bin/bash # Check if the implementation of ComputeConnectedPdaAddress properly handles SPL-specific cases echo "Examining the implementation of ComputeConnectedPdaAddress:" rg -A 10 "func ComputeConnectedPdaAddress" --type go echo "Done."Length of output: 446
Standardized PDA Computation Verified
The updated function call in
e2e/runner/setup_solana.gonow usesComputeConnectedPdaAddress, and verification confirms that its implementation inpkg/contracts/solana/gateway.gocorrectly delegates toComputePdaAddresswith appropriate parameters. This change achieves the intended standardization of PDA address computation across the codebase without any unintended SPL-specific handling remaining.
- File:
pkg/contracts/solana/gateway.go
- Detail: The function
ComputeConnectedPdaAddresscallsComputePdaAddress(connected, []byte("connected")).No further modifications are required.
Description
solana PR for details: zeta-chain/protocol-contracts-solana#88
How Has This Been Tested?
Summary by CodeRabbit