Skip to content

Fixed failing GH actions: nightly tests & populate staging contracts#406

Merged
dusan-maksimovic merged 1 commit intomainfrom
dusan/fix-failing-ci-cycle
Mar 27, 2026
Merged

Fixed failing GH actions: nightly tests & populate staging contracts#406
dusan-maksimovic merged 1 commit intomainfrom
dusan/fix-failing-ci-cycle

Conversation

@dusan-maksimovic
Copy link
Copy Markdown
Collaborator

Description

  • Switched to latest version of Neutron chain binary -> fixes denom resolving for LSMs; Currently our tests are failing because we are using gRPC query that doesn't exist in old version of Neutron chain binary
  • Removed Staging contracts population for Hydro NTRN outpost, since the contract no longer allows creating new lockups, thus making the GH action fail.

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

  • Targeted the correct branch
  • Included the necessary unit tests
  • Added/adjusted the necessary interchain tests
  • Added necessary migration code for all stores that were adjusted or added
  • Added a changelog entry in .changelog
  • Compiled the contracts by using make compile and included content of the artifacts directory into the PR
  • Regenerated front-end schema by using make schema and included generated files into the PR
  • Updated the relevant documentation or specification
  • Reviewed "Files changed" and left comments if necessary
  • Confirmed all CI checks have passed

@dusan-maksimovic dusan-maksimovic self-assigned this Mar 27, 2026
@dusan-maksimovic dusan-maksimovic requested a review from a team as a code owner March 27, 2026 07:19
@dusan-maksimovic dusan-maksimovic merged commit f6e098f into main Mar 27, 2026
1 check passed
@dusan-maksimovic dusan-maksimovic deleted the dusan/fix-failing-ci-cycle branch March 27, 2026 07:19
@claude
Copy link
Copy Markdown

claude bot commented Mar 27, 2026

PR Review

This is a small, focused fix addressing two separate CI failures. The intent is clear and the changes are minimal. A few observations below.


✅ What looks good

  • Both files are updated consistently to use v10.1.0 — no version mismatch between Dockerfile and the workflow.
  • Removing NTRN staging calls is the right call if that contract no longer accepts lockups; keeping dead steps in CI would just add noise and failures.
  • The PR description clearly explains the two motivations.

⚠️ Concerns

1. Major version jump without justification (v5.0.2v10.1.0)

Skipping 5 major versions of neutrond is a significant change. The description mentions fixing a missing gRPC query, but it doesn't explain:

  • What changed in the major versions in between that could affect test behavior?
  • Was v10.1.0 specifically tested, or is it just "latest"? If it's latest today, the next major release could silently break things again.

Consider pinning to a specific tested version and adding a comment referencing the gRPC query that required this update (e.g., what the query name/path is), so future maintainers understand why this exact version is needed.

2. Version defined independently in two places

.github/workflows/populate-staging-contracts.yml:19 and Dockerfile:52 both hardcode v10.1.0 separately. If someone bumps one, they'll need to remember to bump the other. This is a latent consistency risk.

A mitigation would be to read the version from a shared file (e.g., a .neutron-version file or a workflow-level env: block referenced by the Dockerfile ARG), or at least add a comment in each location pointing to the other.

3. Missing changelog entry

The author checklist explicitly leaves the .changelog entry unchecked. According to the project's CLAUDE.md, all PRs should include a changelog entry in .changelog/unreleased/. Even for CI-only fixes, a brief entry under improvements or bug-fixes is appropriate (e.g., "Updated Neutron binary to v10.1.0 to fix gRPC query compatibility").

4. NTRN outpost removal has no follow-up

Removing the NTRN contract staging steps silently drops that outpost from CI coverage. Is this expected to be permanent? If the contract will eventually allow lockups again (e.g., after a migration or upgrade), this removal should be tracked as a known gap with a linked issue. If it's permanent, that context should be documented somewhere.

5. CI pass status not confirmed

The checklist item "Confirmed all CI checks have passed" is unchecked. For a PR specifically fixing CI, it would be good to confirm the actions are now green before merging.


Minor

  • The trailing blank lines removed from the YAML (e.g., after the env variable blocks and script calls) is a nice cleanup — no concerns there.

Summary: The fix is correct and necessary, but the major binary version jump and the silent removal of the NTRN outpost warrant a bit more documentation/tracking. The missing changelog entry should be addressed in a follow-up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant