Skip to content

Conversation

@jpraynaud
Copy link
Member

@jpraynaud jpraynaud commented Jan 30, 2026

Content

This PR includes a fix to:

  • Abort the Pallas DMQ client upon reception of an error (to avoid long lived connections to the DMQ server which could fail)
  • Support the update of the Pallas library that fixes the CBOR encoding for rejected messages in local submission mini-protocol

Pre-submit checklist

  • Branch
    • Tests are provided (if possible)
    • Crates versions are updated (if relevant)
    • Commit sequence broadly makes sense
    • Key commits have useful messages
  • PR
    • All check jobs of the CI have succeeded
    • Self-reviewed the diff
    • Useful pull request description
    • Reviewer requested

Issue(s)

Relates to #2961

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes DMQ publisher connection handling and aligns DMQ rejection encoding with an updated Pallas API to prevent long-lived connections when local submission responses are rejected.

Changes:

  • Update DMQ server rejected-response construction to use DmqMsgRejectReason (new Pallas API).
  • Adjust DMQ publisher client flow around receiving the submit response and sending Done.
  • Switch pallas-* dependencies to a git branch to pick up the CBOR-encoding fix.

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 3 comments.

File Description
internal/mithril-dmq/src/publisher/server/pallas.rs Updates rejected response payload to the new DmqMsgRejectReason-based structure.
internal/mithril-dmq/src/publisher/client/pallas.rs Refactors response handling / graceful termination logic and updates tests for new reject reason type.
internal/mithril-dmq/Cargo.toml Switches pallas-codec / pallas-network from crates.io to a git branch dependency.
Cargo.lock Locks new git-sourced Pallas versions and their transitive dependency changes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +25 to +28
#pallas-codec = { version = "0.34.0" }
#pallas-network = { version = "0.34.0" }
pallas-codec = { git = "https://github.com/cardano-scaling/pallas.git", branch = "jpraynaud/fix-dmq-local-submission" }
pallas-network = { git = "https://github.com/cardano-scaling/pallas.git", branch = "jpraynaud/fix-dmq-local-submission" }
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

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

pallas-codec / pallas-network are switched to a moving git branch dependency. This makes builds non-reproducible over time (branch can change) and bypasses crates.io versioning; it also complicates supply-chain auditing. Prefer pinning to an immutable rev (commit) or a released/tagged version, or use a [patch.crates-io] override at the workspace level if this is a temporary hotfix. Also consider removing the commented-out dependency lines to avoid configuration drift.

Suggested change
#pallas-codec = { version = "0.34.0" }
#pallas-network = { version = "0.34.0" }
pallas-codec = { git = "https://github.com/cardano-scaling/pallas.git", branch = "jpraynaud/fix-dmq-local-submission" }
pallas-network = { git = "https://github.com/cardano-scaling/pallas.git", branch = "jpraynaud/fix-dmq-local-submission" }
pallas-codec = { git = "https://github.com/cardano-scaling/pallas.git", rev = "0123456789abcdef0123456789abcdef01234567" }
pallas-network = { git = "https://github.com/cardano-scaling/pallas.git", rev = "0123456789abcdef0123456789abcdef01234567" }

Copilot uses AI. Check for mistakes.
@github-actions
Copy link

Test Results

    5 files  ±0    172 suites  ±0   33m 19s ⏱️ +28s
2 452 tests ±0  2 452 ✅ ±0  0 💤 ±0  0 ❌ ±0 
7 654 runs  ±0  7 654 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit e61573f. ± Comparison against base commit 2f5a13f.

Copy link
Collaborator

@turmelclem turmelclem left a comment

Choose a reason for hiding this comment

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

LGTM 👍

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.

4 participants