fix: bump acp core to fix policy marshaling non-determinism#112
Conversation
📝 WalkthroughWalkthroughThis PR updates dependencies on ACP and RaccoonDB modules and introduces an integration test validating determinism of ACP policy creation. The test repeatedly initializes commit multistores and verifies that calling ChangesPolicy Determinism Validation
🎯 2 (Simple) | ⏱️ ~10 minutes 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: 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
`@tests/integration/acp/suite/policy/policy_creation_store_determinism_test.go`:
- Line 78: The capability memory store is created and mounted with the wrong
types: replace the call to storetypes.NewKVStoreKey("capkeepermem") with
storetypes.NewMemoryStoreKey("capkeepermem") (symbol: capabilityMemStoreKey) and
when mounting the store (where StoreTypeDB and db are currently used) change to
storetypes.StoreTypeMemory and nil so the memory store uses the correct
in-memory mount; update the mount call that references capKeeperMem/key to use
StoreTypeMemory,nil accordingly.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c9092ee8-8ae1-4b16-b5ea-67bb6e384190
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (2)
go.modtests/integration/acp/suite/policy/policy_creation_store_determinism_test.go
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (2)
tests/integration/acp/suite/policy/policy_creation_store_determinism_test.go (1)
29-71: LGTM!go.mod (1)
50-51: Dependency versions are valid and resolvable.The updated pseudo-versions for acp_core and raccoondb point to legitimate commits and resolve without errors. While these are very recent pseudo-versions, they follow proper Go conventions and pass module validation. For additional security assurance, consider running
govulncheckas part of pre-merge checks if not already in your CI pipeline.
|
|
||
| acpStoreKey := storetypes.NewKVStoreKey(types.StoreKey) | ||
| capabilityStoreKey := storetypes.NewKVStoreKey("capkeeper") | ||
| capabilityMemStoreKey := storetypes.NewKVStoreKey("capkeepermem") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Search for capability keeper initialization patterns to verify correct store type usage
echo "=== Searching for capability keeper store setup patterns ==="
rg -n -A5 -B5 'capabilitykeeper\.NewKeeper' --type=go
echo -e "\n=== Searching for memory store key creation ==="
rg -n 'NewMemoryStoreKey.*cap' --type=go
echo -e "\n=== Searching for StoreTypeMemory usage ==="
rg -n 'StoreTypeMemory' --type=goRepository: sourcenetwork/sourcehub
Length of output: 7169
🏁 Script executed:
cd tests/integration/acp/suite/policy && head -100 policy_creation_store_determinism_test.go | tail -35Repository: sourcenetwork/sourcehub
Length of output: 1418
Fix capability memory store to use correct store types.
Line 78 should use NewMemoryStoreKey instead of NewKVStoreKey, and line 84 should mount it with StoreTypeMemory and nil instead of StoreTypeDB and db. The pattern is correctly used elsewhere in the codebase (e.g., x/acp/capability/manager_test.go lines 21–26).
Proposed fix
acpStoreKey := storetypes.NewKVStoreKey(types.StoreKey)
capabilityStoreKey := storetypes.NewKVStoreKey("capkeeper")
- capabilityMemStoreKey := storetypes.NewKVStoreKey("capkeepermem")
+ capabilityMemStoreKey := storetypes.NewMemoryStoreKey("capkeepermem")
db := dbm.NewMemDB()
cms := store.NewCommitMultiStore(db, log.NewNopLogger(), metrics.NewNoOpMetrics())
cms.MountStoreWithDB(acpStoreKey, storetypes.StoreTypeDB, db)
cms.MountStoreWithDB(capabilityStoreKey, storetypes.StoreTypeDB, db)
- cms.MountStoreWithDB(capabilityMemStoreKey, storetypes.StoreTypeDB, db)
+ cms.MountStoreWithDB(capabilityMemStoreKey, storetypes.StoreTypeMemory, nil)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| capabilityMemStoreKey := storetypes.NewKVStoreKey("capkeepermem") | |
| acpStoreKey := storetypes.NewKVStoreKey(types.StoreKey) | |
| capabilityStoreKey := storetypes.NewKVStoreKey("capkeeper") | |
| capabilityMemStoreKey := storetypes.NewMemoryStoreKey("capkeepermem") | |
| db := dbm.NewMemDB() | |
| cms := store.NewCommitMultiStore(db, log.NewNopLogger(), metrics.NewNoOpMetrics()) | |
| cms.MountStoreWithDB(acpStoreKey, storetypes.StoreTypeDB, db) | |
| cms.MountStoreWithDB(capabilityStoreKey, storetypes.StoreTypeDB, db) | |
| cms.MountStoreWithDB(capabilityMemStoreKey, storetypes.StoreTypeMemory, nil) |
🤖 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 `@tests/integration/acp/suite/policy/policy_creation_store_determinism_test.go`
at line 78, The capability memory store is created and mounted with the wrong
types: replace the call to storetypes.NewKVStoreKey("capkeepermem") with
storetypes.NewMemoryStoreKey("capkeepermem") (symbol: capabilityMemStoreKey) and
when mounting the store (where StoreTypeDB and db are currently used) change to
storetypes.StoreTypeMemory and nil so the memory store uses the correct
in-memory mount; update the mount call that references capKeeperMem/key to use
StoreTypeMemory,nil accordingly.
1 participant
No description provided.