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

enhancement: compare fuel-indexer and forc-index versions #1400

Merged
merged 6 commits into from
Oct 11, 2023

Conversation

lostman
Copy link
Contributor

@lostman lostman commented Oct 6, 2023

Description

Closes #1377.

This PR embeds TOOLCHAIN_VERSION in the indexer's WASM module and adds a check when the indexer is deploys which only permits the indexers whose TOOLCHAIN_VERSION matches the fuel-indexer version.

Testing steps

CI tests should pass.

Manual testing:

  1. Start fuel-indexer:
cargo run -p fuel-indexer -- run --fuel-node-host beta-4.fuel.network --fuel-node-port 80 --replace-indexer
  1. Create and deploy an indexer using an older version of forc-index:
forc-index --version
forc index 0.20.11
forc-index new /tmp/test-indexer
forc-index deploy --path /tmp/some-indexer
▹▸▹▹▹ ⏰ Building indexer...
Finished release [optimized] target(s) in 0.10s
▪▪▪▪▪ ✅ Build succeeded.
Deploying indexer...
▸▹▹▹▹ 🚀 Deploying...
{
  "details": "WASM module toolchain version `unknown` does not match fuel-indexer version `0.21.1`",
  "success": "false"
}

Additionally, run fuel-indexer with --disable-toolchain-version-check to be able to deploy the above indexer anyway.

  1. Deploy one of the indexers in the repository (hence, using the same toolchain version):
cargo run --bin forc-index -- deploy --path examples/fuel-explorer/fuel-explorer/
▹▹▸▹▹ ⏰
Building indexer...
    Finished release [optimized] target(s) in 0.40s
▪▪▪▪▪ ✅ Build succeeded.
Deploying indexer...
▪▪▪▪▪ ✅ Successfully deployed indexer.

Changelog

  • embed TOOLCHAIN_VERSION in WASM module
  • check that the embedded version matches fuel-indexer's version
  • add a flag to disable this behavior

@lostman lostman self-assigned this Oct 6, 2023
@lostman lostman force-pushed the maciej/1377-forc-indexer-version-check branch 6 times, most recently from b399d40 to 167ce4b Compare October 9, 2023 20:13
@lostman lostman force-pushed the maciej/1377-forc-indexer-version-check branch from f969415 to c552b70 Compare October 10, 2023 11:57
@lostman lostman marked this pull request as ready for review October 10, 2023 16:49
ra0x3
ra0x3 previously approved these changes Oct 10, 2023
Copy link
Contributor

@ra0x3 ra0x3 left a comment

Choose a reason for hiding this comment

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

utACK. Will defer to @deekerno

packages/fuel-indexer-api-server/src/ffi.rs Show resolved Hide resolved
packages/fuel-indexer-api-server/src/uses.rs Outdated Show resolved Hide resolved
packages/fuel-indexer-api-server/src/ffi.rs Show resolved Hide resolved
Co-authored-by: rashad <spam.rashad@protonmail.com>
Copy link
Contributor

@ra0x3 ra0x3 left a comment

Choose a reason for hiding this comment

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

Again deferring to @deekerno

@lostman lostman merged commit c638f2d into develop Oct 11, 2023
19 checks passed
@lostman lostman deleted the maciej/1377-forc-indexer-version-check branch October 11, 2023 13:48
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.

Return 405 onforc index deploy unless forc-index version matches fuel-indexer version
3 participants