Skip to content

Conversation

@batconjurer
Copy link
Collaborator

Describe your changes

Closes #4714

  • When dry-running, dummy signatures are used in txs
  • Validation does not perform signature checks when dry-running

I have tested that the gas estimation hasn't changed between dry-running with signatures and with dummies. It is curious however that in both cases there is a small discrepancy between dry-running and the actual tx.

Checklist before merging

  • If this PR has some consensus breaking changes, I added the corresponding breaking:: labels
    • This will require 2 reviewers to approve the changes
  • If this PR requires changes to the docs or specs, a corresponding PR is opened in the namada-docs repo
    • Relevant PR if applies:
  • If this PR affects services such as namada-indexer or namada-masp-indexer, a corresponding PR is opened in that repo
    • Relevant PR if applies:

@batconjurer batconjurer changed the title Bat/dry run mock sigs Dry run with mock sigs Sep 23, 2025
@github-actions github-actions bot added the breaking:api public API breaking change label Sep 23, 2025
@mergify
Copy link
Contributor

mergify bot commented Sep 23, 2025

🧪 CI Insights

Here's what we observed from your CI run for 0b72a14.

✅ Passed Jobs With Interesting Signals

Pipeline Job Signal Health on main Retries 🔍 CI Insights 📄 Logs
Lint 🧹 Build 🛠️ Test 🚦 test-e2e (2) Base branch is broken, but the job passed. Looks like this might be a real fix 💪 Broken 0 View View
test-e2e (3) Base branch is broken, but the job passed. Looks like this might be a real fix 💪 Broken 0 View View
test-e2e-with-device-automation Base branch is broken, but the job passed. Looks like this might be a real fix 💪 Broken 0 View View
test-integration Base branch is broken, but the job passed. Looks like this might be a real fix 💪 Broken 0 View View
test-unit Base branch is broken, but the job passed. Looks like this might be a real fix 💪 Broken 0 View View

@tzemanovic
Copy link
Collaborator

The changes look great but I think we'll need to find where the discrepancy comes from as we need this feature for more accurate gas estimation

@batconjurer
Copy link
Collaborator Author

The changes look great but I think we'll need to find where the discrepancy comes from as we need this feature for more accurate gas estimation

I'm not sure how this feature is supposed to give more accurate gas estimation unless you mean that we can now dry-run txs that require a hardware wallet to sign. In any case, this discrepancy existed before these changes and it seems from my limited testing that these changes preserve the values for both the dry-run and actual run. The difference is like 100 - 200 gas units, and the dry-runs are higher than the actual.

@tzemanovic
Copy link
Collaborator

The changes look great but I think we'll need to find where the discrepancy comes from as we need this feature for more accurate gas estimation

I'm not sure how this feature is supposed to give more accurate gas estimation unless you mean that we can now dry-run txs that require a hardware wallet to sign. In any case, this discrepancy existed before these changes and it seems from my limited testing that these changes preserve the values for both the dry-run and actual run. The difference is like 100 - 200 gas units, and the dry-runs are higher than the actual.

Ah, it's aimed for the interface where we need to estimate gas cost before signing, mostly for the txs that have very variable gas cost depending on the execution path (e.g. PoS redelegation). But if the dry-run estimates are a bit higher I think that's acceptable

@tzemanovic tzemanovic added merge Ready to merge - mergifyio bot will add the PR to merge queue when all checks pass backport-201.0 Backport to app 201.0 maintenance branch labels Sep 24, 2025
mergify bot added a commit that referenced this pull request Sep 24, 2025
@tzemanovic tzemanovic mentioned this pull request Sep 24, 2025
3 tasks
@mergify
Copy link
Contributor

mergify bot commented Sep 24, 2025

This pull request has been removed from the queue for the following reason: checks failed.

The merge conditions cannot be satisfied due to failing checks:

You can check the last failing draft PR here: #4838.

You may have to fix your CI before adding the pull request to the queue again.
If you update this pull request, to fix the CI, it will automatically be requeued once the queue conditions match again.
If you think this was a flaky issue instead, you can requeue the pull request, without updating it, by posting a @mergifyio requeue comment.

@mergify
Copy link
Contributor

mergify bot commented Sep 24, 2025

Hey @batconjurer, your pull request has been dequeued due to the following reason: CHECKS_FAILED.
Sorry about that, but you can requeue the PR by using @mergifyio requeue if you think this was a mistake.

@tzemanovic
Copy link
Collaborator

@Mergifyio requeue

@mergify
Copy link
Contributor

mergify bot commented Sep 24, 2025

requeue

✅ The queue state of this pull request has been cleaned. It can be re-embarked automatically

mergify bot added a commit that referenced this pull request Sep 24, 2025
@mergify
Copy link
Contributor

mergify bot commented Sep 24, 2025

This pull request has been removed from the queue for the following reason: checks failed.

The merge conditions cannot be satisfied due to failing checks:

You can check the last failing draft PR here: #4840.

You may have to fix your CI before adding the pull request to the queue again.
If you update this pull request, to fix the CI, it will automatically be requeued once the queue conditions match again.
If you think this was a flaky issue instead, you can requeue the pull request, without updating it, by posting a @mergifyio requeue comment.

@mergify
Copy link
Contributor

mergify bot commented Sep 24, 2025

Hey @batconjurer, your pull request has been dequeued due to the following reason: CHECKS_FAILED.
Sorry about that, but you can requeue the PR by using @mergifyio requeue if you think this was a mistake.

@tzemanovic
Copy link
Collaborator

@Mergifyio requeue

@mergify
Copy link
Contributor

mergify bot commented Sep 24, 2025

requeue

✅ The queue state of this pull request has been cleaned. It can be re-embarked automatically

mergify bot added a commit that referenced this pull request Sep 24, 2025
@mergify
Copy link
Contributor

mergify bot commented Sep 24, 2025

This pull request has been removed from the queue for the following reason: checks failed.

The merge conditions cannot be satisfied due to failing checks:

You can check the last failing draft PR here: #4841.

You may have to fix your CI before adding the pull request to the queue again.
If you update this pull request, to fix the CI, it will automatically be requeued once the queue conditions match again.
If you think this was a flaky issue instead, you can requeue the pull request, without updating it, by posting a @mergifyio requeue comment.

@mergify
Copy link
Contributor

mergify bot commented Sep 24, 2025

Hey @batconjurer, your pull request has been dequeued due to the following reason: CHECKS_FAILED.
Sorry about that, but you can requeue the PR by using @mergifyio requeue if you think this was a mistake.

@tzemanovic
Copy link
Collaborator

@Mergifyio requeue

@mergify
Copy link
Contributor

mergify bot commented Sep 25, 2025

requeue

✅ The queue state of this pull request has been cleaned. It can be re-embarked automatically

@mergify
Copy link
Contributor

mergify bot commented Sep 26, 2025

This pull request has been removed from the queue for the following reason: checks failed.

The merge conditions cannot be satisfied due to failing checks:

You can check the last failing draft PR here: #4869.

You may have to fix your CI before adding the pull request to the queue again.
If you update this pull request, to fix the CI, it will automatically be requeued once the queue conditions match again.
If you think this was a flaky issue instead, you can requeue the pull request, without updating it, by posting a @mergifyio requeue comment.

@mergify mergify bot removed the queued label Sep 26, 2025
@mergify
Copy link
Contributor

mergify bot commented Sep 26, 2025

Hey @batconjurer, your pull request has been dequeued due to the following reason: CHECKS_FAILED.
Sorry about that, but you can requeue the PR by using @mergifyio requeue if you think this was a mistake.

@tzemanovic
Copy link
Collaborator

@Mergifyio requeue

@mergify
Copy link
Contributor

mergify bot commented Sep 26, 2025

requeue

✅ The queue state of this pull request has been cleaned. It can be re-embarked automatically

@mergify mergify bot added the queued label Sep 26, 2025
mergify bot added a commit that referenced this pull request Sep 26, 2025
mergify bot added a commit that referenced this pull request Sep 26, 2025
@mergify
Copy link
Contributor

mergify bot commented Sep 26, 2025

This pull request has been removed from the queue for the following reason: checks failed.

The merge conditions cannot be satisfied due to failing checks:

You can check the last failing draft PR here: #4870.

You may have to fix your CI before adding the pull request to the queue again.
If you update this pull request, to fix the CI, it will automatically be requeued once the queue conditions match again.
If you think this was a flaky issue instead, you can requeue the pull request, without updating it, by posting a @mergifyio requeue comment.

@mergify mergify bot removed the queued label Sep 26, 2025
@mergify
Copy link
Contributor

mergify bot commented Sep 26, 2025

Hey @batconjurer, your pull request has been dequeued due to the following reason: CHECKS_FAILED.
Sorry about that, but you can requeue the PR by using @mergifyio requeue if you think this was a mistake.

@Fraccaman
Copy link
Collaborator

@Mergifyio requeue

@mergify
Copy link
Contributor

mergify bot commented Sep 26, 2025

requeue

✅ The queue state of this pull request has been cleaned. It can be re-embarked automatically

@mergify mergify bot added the queued label Sep 26, 2025
mergify bot added a commit that referenced this pull request Sep 26, 2025
mergify bot added a commit that referenced this pull request Sep 27, 2025
@mergify mergify bot merged commit 0a456c6 into main Sep 27, 2025
27 checks passed
@mergify mergify bot removed the queued label Sep 27, 2025
@mergify mergify bot deleted the bat/dry-run-mock-sigs branch September 27, 2025 10:15
@mergify mergify bot mentioned this pull request Sep 27, 2025
3 tasks
mergify bot added a commit that referenced this pull request Sep 28, 2025
@tzemanovic tzemanovic added the backport-libs-0.251 Backport libraries to 0.251 maintenance branch label Oct 8, 2025
@tzemanovic
Copy link
Collaborator

I'm backporting this to libs as #4816 depends on it for some mocking fns

@tzemanovic tzemanovic restored the bat/dry-run-mock-sigs branch October 8, 2025 16:20
@mergify mergify bot mentioned this pull request Oct 8, 2025
3 tasks
mergify bot added a commit that referenced this pull request Oct 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-201.0 Backport to app 201.0 maintenance branch backport-libs-0.251 Backport libraries to 0.251 maintenance branch breaking:api public API breaking change merge Ready to merge - mergifyio bot will add the PR to merge queue when all checks pass

Projects

None yet

Development

Successfully merging this pull request may close these issues.

allow to dry-run txs with fake signatures

5 participants