Only allow creation of single denom vault#241
Conversation
|
Warning Review limit reached
Next review available in: 40 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
📝 WalkthroughWalkthroughVault creation now enforces a single payment denom matching the underlying asset and rejects ChangesSingle-denom vault creation
Pause event documentation
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant MsgCreateVaultRequest
participant KeeperCreateVault
participant VaultState
Client->>MsgCreateVaultRequest: submit vault creation
MsgCreateVaultRequest->>KeeperCreateVault: validate single-denom request
KeeperCreateVault->>VaultState: persist vault without creation NAV seed
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
|
| Filename | Overview |
|---|---|
| keeper/vault.go | Adds keeper-level checks for single-denom vault creation and removes initial payment NAV seeding. |
| types/msgs.go | Updates create-vault stateless validation to reject mixed-denom creation and any initial payment NAV. |
| proto/provlabs/vault/v1/tx.proto | Marks the old payment-denom and initial-payment-NAV request fields as deprecated while preserving their field numbers. |
| simulation/operations.go | Updates simulated vault creation to use only empty payment denom or payment equal to the underlying asset. |
| simulation/vault.go | Removes simulation helper seeding for initial payment NAV on newly created vaults. |
Reviews (2): Last reviewed commit: "add deprecated docs to protos" | Re-trigger Greptile
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@api/provlabs/vault/v1/events.pulsar.go`:
- Around line 25902-25918: The documentation for the pause event fields was
edited in generated `events.pulsar.go` output and will be overwritten. Move the
comments for `Reason`, `TotalVaultValue`, `Forced`, and `ForcedError` to their
corresponding fields in the protobuf source, then run `make proto-all` to
regenerate the API code.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 875c1018-321c-414c-9725-dd0aba783a4d
⛔ Files ignored due to path filters (2)
types/events.pb.gois excluded by!**/*.pb.gotypes/tx.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (19)
.changelog/unreleased/improvements/239-disallow-multi-denom-vault.mdapi/provlabs/vault/v1/events.pulsar.goapi/provlabs/vault/v1/tx.pulsar.gokeeper/query_server_test.gokeeper/reconcile_test.gokeeper/suite_test.gokeeper/vault.gokeeper/vault_test.goproto/provlabs/vault/v1/tx.protosimulation/estimate_accuracy_test.gosimulation/operations.gosimulation/operations_test.gosimulation/setup.gosimulation/vault.gospec/01_concepts.mdspec/02_state.mdspec/03_messages.mdtypes/msgs.gotypes/msgs_test.go
💤 Files with no reviewable changes (1)
- simulation/vault.go
| // reason is the reason for pausing the vault. For a manual pause (PauseVault | ||
| // tx) this is the user-supplied reason. For an automated auto-pause triggered | ||
| // in the begin/end blocker, this carries the hard-coded reason describing the | ||
| // critical error that forced the pause. | ||
| Reason string `protobuf:"bytes,3,opt,name=reason,proto3" json:"reason,omitempty"` | ||
| // total_vault_value is the total value of the vault's assets at the time of pausing. | ||
| TotalVaultValue string `protobuf:"bytes,4,opt,name=total_vault_value,json=totalVaultValue,proto3" json:"total_vault_value,omitempty"` | ||
| // forced indicates the pause used the force path, waiving the strict | ||
| // reconcile/valuation gate. False for a normal strict pause; true for a | ||
| // forced manual pause and for automated auto-pauses. | ||
| Forced bool `protobuf:"varint,5,opt,name=forced,proto3" json:"forced,omitempty"` | ||
| // forced_error records the reconcile and/or valuation error that was | ||
| // tolerated when forced is true. Empty when nothing failed. When non-empty, | ||
| // total_vault_value may be stale or zero. | ||
| // forced_error records the reconcile and/or valuation error tolerated by a | ||
| // manual force pause (PauseVault tx). Empty when nothing failed. When | ||
| // non-empty, total_vault_value may be stale or zero. Only the tx path sets | ||
| // this; automated auto-pauses leave it empty and instead encode their error | ||
| // in reason. | ||
| ForcedError string `protobuf:"bytes,6,opt,name=forced_error,json=forcedError,proto3" json:"forced_error,omitempty"` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Update the protobuf source instead of editing generated API code.
These comments are modified directly in api/provlabs/vault/v1/events.pulsar.go, so the next make proto-all may overwrite them. Move the documentation to the corresponding .proto fields, then regenerate the API output.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@api/provlabs/vault/v1/events.pulsar.go` around lines 25902 - 25918, The
documentation for the pause event fields was edited in generated
`events.pulsar.go` output and will be overwritten. Move the comments for
`Reason`, `TotalVaultValue`, `Forced`, and `ForcedError` to their corresponding
fields in the protobuf source, then run `make proto-all` to regenerate the API
code.
Source: Coding guidelines
|
We are just removing multi-denom completely now with PR #243 |
1 participant
closes: #239
Summary by CodeRabbit
New Features
Documentation