Skip to content

chore(dashmate): set tenderdash version to 1 #2385

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Dec 12, 2024
Merged

chore(dashmate): set tenderdash version to 1 #2385

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/dashmate/configs/defaults/getBaseConfigFactory.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ export default function getBaseConfigFactory() {
tenderdash: {
mode: 'full',
docker: {
image: 'dashpay/tenderdash:1.3',
image: 'dashpay/tenderdash:1',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still believe this should be 1.4. Using 1 can cause use of - for example - 1.0.0 (if it's already pulled) which is not compatible.

We had issues with drive where people didn't do dashmate update (or it failed?) and were reporting issues. Using more precise tag should fix this issue, and enforce update even if the user doesn't call dashmate update directly.

And it will protect against unintended minor version updates.

Copy link
Collaborator Author

@shumkov shumkov Dec 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We use the same approach for Core (only major version is specified).
Will you change your opinion if dashmate will automatically pull images after updating the package version?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will definitely help, but will still not be perfect. The issue is that docker does not support semver :)

Accepting for now, but needs more discussion.

},
p2p: {
host: '0.0.0.0',
Expand Down
7 changes: 7 additions & 0 deletions packages/dashmate/configs/getConfigFileMigrationsFactory.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1048,6 +1048,13 @@ export default function getConfigFileMigrationsFactory(homeDir, defaultConfigs)
});
return configFile;
},
'1.7.0': (configFile) => {
Object.entries(configFile.configs)
.forEach(([, options]) => {
options.platform.drive.tenderdash.docker.image = 'dashpay/tenderdash:1';
});
return configFile;
},
Comment on lines +1051 to +1057
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Migration introduces version regression from 1.3 to 1

The version history shows a concerning regression in tenderdash versions:

  • Version 1.1.1: Sets to 'dashpay/tenderdash:1.2.0'
  • Version 1.3.0: Updates to 'dashpay/tenderdash:1.3'
  • Version 1.4.0-dev.1: Maintains 'dashpay/tenderdash:1.3'
  • Version 1.7.0 (current): Downgrades to 'dashpay/tenderdash:1'

This migration appears to be downgrading the tenderdash version from 1.3 to 1, which is likely unintended and could cause compatibility issues. The version should either maintain 1.3 or upgrade to a newer version, but not downgrade.

🔗 Analysis chain

LGTM! Migration function correctly updates tenderdash version.

The migration function follows the established pattern and properly updates the tenderdash Docker image version to '1' across all configurations, which aligns with the PR objectives.

Let's verify that this is the only place where the tenderdash version needs to be updated:

Tenderdash version update requires additional changes

The migration function needs to be reviewed as there are inconsistencies in tenderdash versioning across the codebase:

  • packages/dashmate/configs/defaults/getBaseConfigFactory.js sets default image to 'dashpay/tenderdash:1'
  • Multiple migrations in getConfigFileMigrationsFactory.js show version changes:
    • From 0.13.2 (in test fixtures) to 1.2.0
    • Then to 1.3
    • And now attempting to change to 1

This suggests a potential regression as we're moving from version 1.3 to 1, which might not be intended. Let's verify the version history:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for other occurrences of tenderdash version specifications
# Test: Search for tenderdash Docker image version declarations
rg "dashpay/tenderdash:[0-9]" --type js

Length of output: 1334

Script:

#!/bin/bash
# Get the chronological order of migrations to understand version progression
ast-grep --pattern 'options.platform.drive.tenderdash.docker.image = $_' packages/dashmate/configs/getConfigFileMigrationsFactory.js -A 5 -B 5

Length of output: 25532

};
}

Expand Down
Loading