fix(dashmate): re-sync stale 3.x Drive and rs-dapi images#3898
fix(dashmate): re-sync stale 3.x Drive and rs-dapi images#3898thepastaclaw wants to merge 4 commits into
Conversation
📝 WalkthroughWalkthroughAdds a Changes4.0.0-rc.3 Docker image migration and tests
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
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 |
|
@coderabbitai review |
✅ Action performedReview finished.
|
![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 `@packages/dashmate/test/unit/config/configFile/migrate3xTo4xRcImages.spec.js`:
- Around line 26-31: The test is missing coverage for the `:3-rc` image variant
even though the migration predicate matches it alongside `dev` and `hotfix`. Add
constants for the rc variants (STALE_DRIVE_RC and STALE_RS_DAPI_RC) following
the same pattern as the existing STALE_DRIVE_DEV and STALE_RS_DAPI_DEV
constants. Then extend the test matrix around line 142-145 to include test cases
for these new rc variant constants, ensuring the migration correctly handles the
rc versions just as it does for dev and hotfix variants.
🪄 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: 1ea52087-7c6e-4e68-9174-d84d8fca3052
📒 Files selected for processing (2)
packages/dashmate/configs/getConfigFileMigrationsFactory.jspackages/dashmate/test/unit/config/configFile/migrate3xTo4xRcImages.spec.js
|
@coderabbitai review |
✅ Action performedReview finished.
|
2ce01c6 to
e11795d
Compare
|
✅ Final review complete — no blockers (commit ced758c) |
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Targeted dashmate migration that re-keys the 3.x→4.0.0-rc image re-sync to 4.0.0-rc.3 (since the previous 4.0.0-rc.2 keying was a no-op for users already on rc.2) and extends coverage to the :3-hotfix tag series. Both agents independently found no in-scope correctness, security, or coverage issues, and the regression test file gives thorough coverage of the prerelease label series plus custom/missing-config edge cases.
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
e11795d to
086187c
Compare
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Prior review at e11795d had zero findings, so there is nothing to carry forward. Both agents independently re-reviewed head 086187c and reported no in-scope issues. Verified directly: the 4.0.0-rc.3 migration uses anchored regexes ^dashpay/drive:3(?:-(?:dev|rc|hotfix))?$ and ^dashpay/rs-dapi:3(?:-(?:dev|rc|hotfix))?$ to limit rewrites to the four dashmate-shipped stale 3.x tag series, gates writes on defaultConfig.has(...), uses optional chaining to tolerate missing dapi/rsDapi sections, and is re-keyed at 4.0.0-rc.3 to escape the same-version short-circuit from the original #3889 attempt. Scope matches the stated goal; no findings.
…c upgrade (dashpay#3889) A node already on 3.0.1 skipped the 3.0.0 migration that used to re-sync the Drive ABCI and rs-dapi docker images, and the intervening 3.0.1 / 3.0.2 / 3.1.0 migrations only touched Core / Gateway / Tenderdash. After `dashmate update` to 4.0.0-rc.x the config kept the old protocol-11 images (`dashpay/drive:3`, `dashpay/rs-dapi:3`), so the node stayed on protocol 11 and crash-looped after protocol 12 activation. Add a `4.0.0-rc.2` migration that re-syncs both image fields from the current default config (matching the 3.0.0 migration convention), and a focused regression spec that constructs the migration without the full DI container so it doesn't depend on `@dashevo/wasm-dpp` being built. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Apply review polish on the regression added in 686e995: - Match the sibling 3.0.0 migration's single-line `defaultConfig.get(...)` style in the migration body. - Hoist expected default images into `beforeEach`; remove an unused `customImages` helper parameter. - Parameterise the two `fromVersion` cases (3.0.1 and 3.1.0) so both real-world entry points are exercised by one assertion shape. - Pin against the regression with `expect(...).to.not.equal('dashpay/drive:3')` (and rs-dapi:3) so the test keeps failing even if a future default happens to match the stale value again. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…custom images The 4.0.0-rc.2 entry added in dashpay#3889 was both unreachable for already- stamped rc.2 configs (migrateConfigFile short-circuits when fromVersion === toVersion) and unconditional: it overwrote every Drive ABCI / rs-dapi image with the current default, clobbering private forks and vendor-patched builds. Re-key the migration at 4.0.0-rc.3 (the next release; same flow as the 3.0.2 Envoy CVE - dashpay#3794 added the migration, dashpay#3796 cut the release) and only rewrite the stale dashmate-shipped tags (dashpay/drive:3, dashpay/rs-dapi:3, including their -dev/-rc series), mirroring the 3.0.2 Envoy CVE migration's predicate style. Custom images are preserved. The migration fires for both 3.0.x -> 4.0.0-rc.x upgrades (the loop runs every migration with key > fromVersion) and 4.0.0-rc.2 -> 4.0.0-rc.3 once a chore(release) bumps the package version. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous predicate alternation only matched the `-dev` and `-rc`
prerelease label series for `dashpay/drive:3` / `dashpay/rs-dapi:3`,
missing the `-hotfix` tag shipped by 3.0.1-hotfix.{1..4} (dashpay#3020/dashpay#3044/
dashpay#3055/dashpay#3060) and 3.1.0-hotfix.1. A node sitting on `:3-hotfix` would
still crash-loop after protocol 12 activation — the exact failure mode
the migration was meant to prevent. Extend the alternation and add a
regression case.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
086187c to
ced758c
Compare
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Carried-forward prior findings: none; the prior verified output was clean and there is nothing to reconcile. New findings in the latest delta: the Tenderdash custom-image clobber is real in the checked-out code, but it was introduced by upstream base commit 1157ca7 (#3940), not by PR #3898's diff against the current base, so there are no in-scope findings for this PR.
|
@coderabbitai review |
✅ Action performedReview finished.
|
✅ Action performedReview finished.
|
✅ Action performedReview finished.
|
✅ Action performedReviews resumed. |
2 participants
Issue being fixed or feature implemented
Fixes #3889.
Dashmate configs upgraded from the 3.x line to 4.0.0-rc.2 could keep stale protocol-11 Drive ABCI and rs-dapi Docker image tags (
dashpay/drive:3*,dashpay/rs-dapi:3*). Those images crash-loop after protocol 12 activation.What was done?
4.0.0-rc.3so configs already stamped4.0.0-rc.2can be repaired on the next release bump instead of being skipped by the same-version short-circuit.:3,:3-dev,:3-rc, and:3-hotfix.How Has This Been Tested?
Environment:
/Users/claw/Projects/platform/worktrees/fix-dashmate-rc-image-migrationnpx -p node@22because local system Node 25 hits an EBADF loader issue before the spec runs.Validation:
npx -y -p node@22 node .yarn/releases/yarn-4.12.0.cjs workspace dashmate exec mocha test/unit/config/configFile/migrate3xTo4xRcImages.spec.jsResult:
11 passing (87ms).Pre-PR review gate:
Result:
Recommendation: ship.Breaking Changes
None.
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit