Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

chore: support RPC consistency mechanism #3741

Draft
wants to merge 44 commits into
base: ns/chore/fuel-core-0.42.0
Choose a base branch
from

Conversation

nedsalk
Copy link
Contributor

@nedsalk nedsalk commented Feb 27, 2025

Release notes

In this release, we:

  • added support for required_fuel_block_height to fix RPC consistency issues

Summary

flowchart TD
    Initialize[init: currentBlockHeight = 0] --> prepareRequest[prepare request]
    
    prepareRequest --> isRequestBlockSensitive[is request block height sensitive]

    isRequestBlockSensitive --> isSensitive[yes]
    isRequestBlockSensitive --> isNotSensitive[no]

    isSensitive --> addBlockHeight[inform required block height]

    addBlockHeight --> sendRequest[send request]

    isNotSensitive --> sendRequest[send request]


    sendRequest --> response[response has block height error?]

    response --> blockHeightError(yes)
    response --> noBlockHeightError(no)

    blockHeightError --> shouldRetry(should retry?)

    shouldRetry --> retry(yes)
    shouldRetry --> noRetry(no)

    retry --> wait(wait)
    wait --> sendRequest

    noRetry --> throwError(throw error)

    noBlockHeightError --> extractBlockHeight(extract block height from response)

    extractBlockHeight --> isHigher

    isHigher[is value higher than current?]

    isHigher --> higher[yes]
    isHigher --> nonHigher[no]

    higher --> updateBlockHeight[set new value]
    updateBlockHeight --> End[end]
    
    nonHigher --> End

Loading

Checklist

  • All changes are covered by tests (or not applicable)
  • All changes are documented (or not applicable)
  • I reviewed the entire PR myself (preferably, on GH UI)
  • I described all Breaking Changes (or there's none)

@nedsalk nedsalk added feat Issue is a feature chore Issue is a chore labels Feb 27, 2025
@nedsalk nedsalk self-assigned this Feb 27, 2025
Copy link

vercel bot commented Feb 27, 2025

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

Name Status Preview Comments Updated (UTC)
fuels-template ❌ Failed (Inspect) Mar 10, 2025 1:07pm
ts-docs ❌ Failed (Inspect) Mar 10, 2025 1:07pm
ts-docs-api ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 10, 2025 1:07pm

@nedsalk nedsalk removed the feat Issue is a feature label Feb 27, 2025
@github-actions github-actions bot added the feat Issue is a feature label Feb 27, 2025
@nedsalk nedsalk changed the title chore: support optimistic concurrency control chore: support RPC consistency mechanism Feb 27, 2025
Copy link
Contributor Author

@nedsalk nedsalk Mar 10, 2025

Choose a reason for hiding this comment

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

For some reason, the Provider static instance isn't the same between the one available in the tests that's initialized in launchTestNode and the one that's used in our runCommand test utility; perhaps it has something to do with comandeer. I verified this by assigning a static id = getRandomB256() to the Provider and logging it in the constructor. A different string gets logged after calling runCommand.

The problem is that both of them live simultaneously across all tests in this file and what would happen is the first test would finish successfully, kill the node and clear the currentBlockHeight cache of its static instance... but it wouldn't clear the other Provider's currentBlockHeight which would remain high (e.g. 15). In the next test, runDeploy would try to deploy to a new fuel-core node initialized for that test, but that node would have a block height of 0 and it would stall as the required_fuel_block_height isn't reached. Our custom awaiting mechanism isn't working here because we're running the node with --poa-instant true so we're only creating blocks when submitting transactions - but we can't submit a transaction because the block height is too low.

In the end, I opted to rewrite the tests to not use the same 4000 port across all tests, which resolves us of the problem. We might only have flakiness in the very rare case that the OS assigns the same port to launchTestNode in two tests in this file, which would reproduce the same problem. Because runCommand is our test utility, it won't affect the users. For users who want to run all their launchTestNode tests on the same port as was done in this test, this change will suffice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Issue is a chore
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants