-
Notifications
You must be signed in to change notification settings - Fork 852
Codexprepare seikinsettlement for deployment #2413
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Codexprepare seikinsettlement for deployment #2413
Conversation
…ting Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
…tegration-tests Add conditional Slinky integration test
Potential fix for code scanning alert no. 422: Potentially unsafe quoting
…caching Improve CI workflows
…tion-with-slack-breoql chore: enhance codex slack review workflow
…tion-with-slack chore: enhance codex slack review workflow
…tion-with-slack chore: enhance codex slack review workflow
…ng-issue Log estimate gas errors
…d-circlecciprouter-contracts-0x9h4c Add Keeper workflow and guard SeiKin settlement flows
…y-seikinsettlement-and-circlecciprouter-contracts
…d-circlecciprouter-contracts Add SeiKin settlement and Circle CCIP router contracts
…re-seikinsettlement-for-deployment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
// GetSigners returns the message signers. | ||
func (m *MsgCommitCovenant) GetSigners() []sdk.AccAddress { | ||
addr, err := sdk.AccAddressFromBech32(m.Creator) | ||
if err != nil { | ||
return []sdk.AccAddress{} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P0] Covenant commits can run with zero signatures
GetSigners
returns an empty slice whenever Creator
fails Bech32 parsing, and ValidateBasic
later returns nil
. A transaction can set an invalid creator, include no signatures, and still invoke SeiNetCommitCovenantSync
, allowing anyone to commit a covenant or trigger royalty enforcement. The message should reject invalid addresses in ValidateBasic
so that at least one signer is required.
Useful? React with 👍 / 👎.
// GetSigners returns the message signers. | ||
func (m *MsgUnlockHardwareKey) GetSigners() []sdk.AccAddress { | ||
addr, err := sdk.AccAddressFromBech32(m.Creator) | ||
if err != nil { | ||
return []sdk.AccAddress{} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P0] Hardware key unlock message skips authentication
Like MsgCommitCovenant
, MsgUnlockHardwareKey
silently returns no signers when the creator address is malformed and ValidateBasic
performs no checks. An attacker can submit a transaction with a bogus creator and zero signatures to mark any key as unlocked. Ensure ValidateBasic
validates the creator field (or return an error in GetSigners
) so the message cannot execute without a valid signer.
Useful? React with 👍 / 👎.
// RegisterMsgServer is a no-op placeholder to satisfy interface in Configurator. | ||
func RegisterMsgServer(s grpc.ServiceRegistrar, srv MsgServer) {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P1] Seinet message server never registered
RegisterMsgServer
is an empty stub, yet AppModule.RegisterServices
calls it to expose the module’s handlers. Because the registrar is never invoked, MsgCommitCovenant
/MsgUnlockHardwareKey
will be rejected with unknown service seinet.Msg
and the module cannot be used over gRPC or standard transactions. Implement real registration (as other modules do) or remove the call.
Useful? React with 👍 / 👎.
Describe your changes and provide context
Testing performed to validate your change