Update vault clone method#172
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughReplaces legacy gogoproto cloning in VaultAccount with a proto-v2-compatible clone via protoadapt + gproto.Clone; InitGenesis now uses the cloned VaultAccount; simulation helpers now require txConfig; dependency/go toolchain bumps applied; changelog entry added; attribute constructor signature updated and callsites adjusted. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@go.mod`:
- Around line 3-5: The go.mod currently declares "go 1.23.2" and a toolchain of
"go1.24.1"; update the toolchain directive to a more recent patch release (e.g.,
change "toolchain go1.24.1" to "toolchain go1.24.12" or to the latest stable
like "toolchain go1.25.6") so the project uses a secure, up-to-date Go toolchain
while leaving the module "go" version unchanged unless you intend to upgrade
language features.
- Line 16: Update the dependency entry for github.com/cometbft/cometbft in
go.mod from v0.38.20 to v0.38.21 (or later) to pull the CRITICAL consensus
vulnerability fix; after editing the line for github.com/cometbft/cometbft, run
the module update commands (e.g., go get github.com/cometbft/cometbft@v0.38.21
&& go mod tidy) and rebuild/run tests to ensure compatibility with any API
changes.
In `@types/vault.go`:
- Around line 93-96: Update the stale comment on the VaultAccount.Clone method
to refer to VaultAccount (not MarkerAccount), and make the return type assertion
defensive: call protoadapt.MessageV1Of(gproto.Clone(protoadapt.MessageV2Of(&v)))
but instead of a naked type assertion to (*VaultAccount) which can panic,
perform a safe comma-ok type assertion and handle the failure (e.g., return nil
or log/error) before returning; reference the VaultAccount.Clone function and
the protoadapt.MessageV1Of / gproto.Clone / protoadapt.MessageV2Of calls when
making the change.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@go.mod`:
- Line 16: Update the cometbft dependency in go.mod by changing the version
string for the module "github.com/cometbft/cometbft" from v0.38.20 to v0.38.21
to pull in the GHSA-c32p-wcqj-j677 fix; after updating run "go mod tidy" and
rebuild/tests to ensure compatibility, and then audit any protocol logic that
relies on block timestamps to verify behavior remains correct with the patched
release.
In `@types/vault.go`:
- Around line 93-96: The comment for the Clone method is incorrect/outdated: it
mentions "MarkerAccount" but the method is defined on VaultAccount. Update the
comment above the VaultAccount.Clone method to refer to VaultAccount (e.g.,
"Clone makes a VaultAccount instance copy.") so the docstring matches the
receiver and method name VaultAccount.Clone.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@keeper/genesis.go`:
- Line 27: The call to SetVaultLookup at k.SetVaultLookup(ctx, v.Clone()) is
ignoring its returned error; change this to capture the error and handle it the
same way as the later call (panic on failure). Specifically, update the code
around that invocation to: err := k.SetVaultLookup(ctx, v.Clone()); if err !=
nil { panic(fmt.Errorf("failed to set vault lookup: %w", err)) } (use the same
panic/message style as the later check) so failures during genesis import are
not silently dropped.
🧹 Nitpick comments (2)
go.mod (1)
5-5: Consider updating the Go toolchain to a more recent patch release.Per previous review feedback, Go 1.24.12 (or Go 1.25.6) includes additional security and bug fixes compared to go1.24.1.
simapp/go.mod (1)
5-5: Consider updating the Go toolchain to a more recent patch release.As noted in the main module, Go 1.24.12 (or Go 1.25.6) includes additional security and bug fixes compared to go1.24.1.
2 participants
closes: #171
Summary by CodeRabbit
Bug Fixes
Chores
Refactor
Tests