Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Ignition: fix for bug when we fail to save transaction hash #6229

Merged
merged 6 commits into from
Feb 21, 2025

Conversation

zoeyTM
Copy link
Contributor

@zoeyTM zoeyTM commented Feb 6, 2025

In this PR

  • added new journal message TRANSACTION_PREPARE_SEND that is written to journal immediately before sending a transaction to the network
  • added new logic to execution engine that checks the deployment state for any execution state that has a nonce but no transactions and throws if it finds one, alerting the user and asking them to find the appropriate transaction on a block explorer and use the new track-tx command to add it to the deployment
  • added a new track-tx command that allows users to add a missing transaction back to their deployment. accounts for several edge cases
  • tests for all of the above

Copy link

changeset-bot bot commented Feb 6, 2025

🦋 Changeset detected

Latest commit: 84a763e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 5 packages
Name Type
@nomicfoundation/ignition-core Patch
@nomicfoundation/ignition-ui Patch
@nomicfoundation/hardhat-ignition Patch
@nomicfoundation/hardhat-ignition-ethers Patch
@nomicfoundation/hardhat-ignition-viem Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@zoeyTM zoeyTM requested a review from alcuadrado February 6, 2025 10:14
Copy link

vercel bot commented Feb 6, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
hardhat ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 12, 2025 7:55am

// case 2: the user sent a different transaction that replaced ours
// so we check their transaction for the required number of confirmations
else {
return checkConfirmations(
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if I understand this part, let's look at it on a call. Other than that LGTM.

Copy link
Member

Choose a reason for hiding this comment

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

self-answer: This works because it's the else to the if in line 104. Hence, the transaction matches the sender and nonce, but not data and value. i.e. a replacement.

Copy link
Member

@alcuadrado alcuadrado left a comment

Choose a reason for hiding this comment

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

Re-reviewed this, and now I get it. I think the else in the newline was confusing me.

@zoeyTM zoeyTM merged commit 76c107b into main Feb 21, 2025
44 checks passed
@zoeyTM zoeyTM deleted the ignition/tx-failure-fix branch February 21, 2025 03:15
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants