Skip to content

Fix transaction re-signing#554

Merged
popenta merged 1 commit intomainfrom
fix-tx-resigning
Oct 6, 2025
Merged

Fix transaction re-signing#554
popenta merged 1 commit intomainfrom
fix-tx-resigning

Conversation

@popenta
Copy link
Collaborator

@popenta popenta commented Oct 6, 2025

No description provided.

@popenta popenta self-assigned this Oct 6, 2025
@andreibancioiu andreibancioiu requested a review from Copilot October 6, 2025 10:43
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

This PR fixes transaction re-signing by properly tracking transaction state changes during the signing process. The change ensures that when a transaction's version or options are modified during the signing process (e.g., for hash signing), these alterations are correctly detected to trigger re-signing.

Key changes:

  • Added deep copy tracking of the initial transaction state before modifications
  • Enhanced alteration detection to compare initial vs modified transaction properties
  • Version bump to 11.2.1

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
pyproject.toml Version bump to 11.2.1 for the bug fix release
multiversx_sdk_cli/cli_shared.py Added transaction state tracking and improved alteration detection logic

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +898 to +902
if initial_tx.version != transaction.version:
altered = True

if initial_tx.options != transaction.options:
altered = True
Copy link

Copilot AI Oct 6, 2025

Choose a reason for hiding this comment

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

The logic checks for changes between initial_tx and transaction before any explicit alterations are made. This creates a redundant check since at this point in the function, transaction should be identical to initial_tx. Consider moving these checks after the explicit alteration logic (lines 904-910) to properly detect all changes.

Copilot uses AI. Check for mistakes.
Copy link
Contributor

@andreibancioiu andreibancioiu left a comment

Choose a reason for hiding this comment

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

Missing CLI test.


altered = _alter_version_and_options_if_provided(
args=args,
initial_tx=initial_tx,
Copy link
Contributor

Choose a reason for hiding this comment

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

Naming inconsistency, tx vs. transaction.


def _alter_version_and_options_if_provided(
args: Any,
initial_tx: Transaction,
Copy link
Contributor

Choose a reason for hiding this comment

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

Naming inconsistency.

@popenta popenta merged commit dd8ebaf into main Oct 6, 2025
12 of 13 checks passed
@popenta popenta deleted the fix-tx-resigning branch October 6, 2025 10:51
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.

3 participants