Skip to content

fix: Re-enable fee_estimate_query example and remove mirror-sync sleeps - #1732

Merged
Dosik13 merged 3 commits into
hiero-ledger:mainfrom
Mounil2005:fix/fee-estimate-query-mirror-sync
Jun 30, 2026
Merged

fix: Re-enable fee_estimate_query example and remove mirror-sync sleeps#1732
Dosik13 merged 3 commits into
hiero-ledger:mainfrom
Mounil2005:fix/fee-estimate-query-mirror-sync

Conversation

@Mounil2005

Copy link
Copy Markdown
Contributor

Description:

Re-enable fee_estimate_query example in run-examples and remove mirror-sync sleeps

The fee_estimate_query example was previously skipped in the run-examples task due to intermittent failures on fresh local mirror node deployments. The underlying issue was that the mirror node hadn't ingested the operator's state when the example ran back-to-back with others.

This PR removes the band-aid time.Sleep() workarounds and re-enables the example by relying on the proper waitForFeeEstimationServiceReady() readiness probe, which polls the FeeEstimationService until it's actually ready to handle queries.

  • Remove fee_estimate_query skip block from Taskfile.yml run-examples task
  • Remove all waitForMirrorNodeSync() calls from fee_estimate_query_e2e_test.go (12 instances)
  • Remove mirrorSyncDelay constant and waitForMirrorNodeSync() helper function
  • Rely on waitForFeeEstimationServiceReady() readiness probe instead of arbitrary sleeps

Related issue(s):

Fixes #1696

Notes for reviewer:

The waitForFeeEstimationServiceReady() function was already in place and working correctly. It probes the FeeEstimationService with a fresh INTRINSIC estimate request every 5 seconds until it succeeds or 10 minutes elapse. This is a proper solution compared to the previous blind 2-second sleep, as it:

  • Doesn't waste time if the service is ready quickly
  • Properly detects when the FeeEstimationService can handle queries
  • Works across all parallel tests with a shared readiness check

Checklist

  • Documented (Issue references and comment block explain the behavior)
  • Tested (Integration tests now run without artificial sleeps and use proper readiness probing)

@lfdt-bot

lfdt-bot commented May 24, 2026

Copy link
Copy Markdown

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@codecov

codecov Bot commented May 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
see 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Mounil2005
Mounil2005 marked this pull request as ready for review May 26, 2026 14:51
@Mounil2005
Mounil2005 requested review from a team as code owners May 26, 2026 14:51
@Mounil2005
Mounil2005 requested review from Dosik13 and agadzhalov May 26, 2026 14:51
@Mounil2005
Mounil2005 marked this pull request as draft May 26, 2026 14:51
@Mounil2005
Mounil2005 force-pushed the fix/fee-estimate-query-mirror-sync branch from b7d212d to 6485279 Compare May 26, 2026 15:04
@Mounil2005
Mounil2005 marked this pull request as ready for review May 29, 2026 08:56

@gsstoykov gsstoykov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't see where waitForFeeEstimationServiceReady is relied upon.

edit: found it mb

@gsstoykov gsstoykov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Have you validated probing this a couple of times. Let's say 10-100?

@Mounil2005

Copy link
Copy Markdown
Contributor Author

Have you validated probing this a couple of times. Let's say 10-100?

Yes waitForFeeEstimationServiceReady uses sync.Once so the probe runs exactly once per test suite regardless of how many parallel tests call it. It actively polls a known-good intrinsic estimate every 5 seconds for up to 10 minutes, only unblocking tests once an actual success response comes back from the mirror node. This is fundamentally different from the old fixed time.Sleep(2s) the tests won't start until the service is genuinely ready, so repeated runs should not flake.

@Mounil2005

Copy link
Copy Markdown
Contributor Author

Pls let me know in case if anything needs to be changed!

- Remove fee_estimate_query skip block from Taskfile.yml run-examples task
- Remove waitForMirrorNodeSync() calls from sdk/fee_estimate_query_e2e_test.go
- Remove mirrorSyncDelay constant and waitForMirrorNodeSync() helper function
- Rely on waitForFeeEstimationServiceReady() readiness probe instead of arbitrary sleeps

Fixes hiero-ledger#1696

Signed-off-by: Mounil Kanakhara <mounilkankhara@gmail.com>
Signed-off-by: Mounil Kanakhara <mounilkankhara@gmail.com>
@Mounil2005
Mounil2005 force-pushed the fix/fee-estimate-query-mirror-sync branch from 3a2d9cf to 4269de2 Compare June 30, 2026 06:50
@Mounil2005

Copy link
Copy Markdown
Contributor Author

@Dosik13 @gsstoykov
Can you pls review this and let me know in case of any changes.

…ample

On a fresh solo deployment the mirror node's FeeEstimationService races the
importer ingesting the genesis fee schedule and returns HTTP 400 until the
scheduled refresh fires. Add the same startup probe used in the integration
tests so the example is robust on fresh local deployments.

Signed-off-by: Mounil Kanakhara <mounilkankhara@gmail.com>

@Dosik13 Dosik13 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm! thank you for the work and the patience!

@Dosik13
Dosik13 merged commit e270eb2 into hiero-ledger:main Jun 30, 2026
13 checks passed
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.

Re-enable fee_estimate_query example in run-examples and remove mirror-sync sleeps

4 participants