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

Update gateway HTTP server object mapping format #3334

Merged
merged 6 commits into from
Jan 6, 2025

Conversation

teor2345
Copy link
Member

@teor2345 teor2345 commented Dec 24, 2024

This PR makes the gateway use the same object mapping format as the RPC server subscription. This is a breaking change.

The format is documented here (it's the same as the result inside the subscription wrapper):
https://github.com/autonomys/subspace/tree/main/crates/subspace-gateway-rpc#using-the-gateway-rpcs

As part of this change, the gateway:

I also made the logging inside the object fetcher more consistent.

Close #3323.

Code contributor checklist:

@teor2345 teor2345 added the improvement it is already working, but can be better label Dec 24, 2024
@teor2345 teor2345 self-assigned this Dec 24, 2024
nazar-pc
nazar-pc previously approved these changes Dec 24, 2024
Copy link
Member

@nazar-pc nazar-pc left a comment

Choose a reason for hiding this comment

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

This would work, but why can't we just make a breaking change here? Is anyone except us using gateway? Can't they change the way they send data when upgrading gateway backend at once?

@teor2345
Copy link
Member Author

This would work, but why can't we just make a breaking change here? Is anyone except us using gateway? Can't they change the way they send data when upgrading gateway backend at once?

@clostao are you happy to just make a breaking change here? It would be simpler for us.

If we did it that way, next time you upgraded the gateway, you'd also need to upgrade to a version of the indexer which sent block numbers as integers (rather than block number strings).

@clostao
Copy link
Member

clostao commented Dec 24, 2024

No problem with that

@teor2345 teor2345 force-pushed the gateway-string-or-int branch from b333b86 to 6699fa6 Compare January 2, 2025 04:20
@teor2345 teor2345 changed the title Accept a string or int in the gateway HTTP server Update gateway HTTP server object mapping format Jan 2, 2025
@teor2345 teor2345 added the breaking-node This PR introduces breaking changes to the node implementation label Jan 2, 2025
@teor2345
Copy link
Member Author

teor2345 commented Jan 2, 2025

If we did it that way, next time you upgraded the gateway, you'd also need to upgrade to a version of the indexer which sent block numbers as integers (rather than block number strings).

We also wanted to make breaking changes to allow batch retrieval of mappings, so we can cache pieces and use them with multiple mappings. So I made all those breaking changes in this PR.

The gateway and indexer request formats are compatible with the existing format. If you want to support multiple hashes, they can be fetched as /{objects|data}/hash1+hash2+hash3. (Single hashes are still just /{objects|data}/hash.)

The indexer response format was:

{
  "hash": "0000000000000000000000000000000000000000000000000000000000000000",
  "pieceIndex": 0,
  "pieceOffset": 0,
  "blockNumber": 0
}

And it's now the same format as the inner part of the RPC subscription. It's more compact to support batches (piece index is first, followed by piece offset), with a version to support future format changes:

{
  "blockNumber": 0,
  "v0": {
    "objects": [
      [
        "0000000000000000000000000000000000000000000000000000000000000000",
        0,
        0
      ]
    ]
  }
}

If that's too much change, let me know, and we'll work out how to do it in stages.

@teor2345
Copy link
Member Author

teor2345 commented Jan 2, 2025

@clostao just checking these changes work for you?

@clostao
Copy link
Member

clostao commented Jan 6, 2025

Looks good to me

@teor2345 teor2345 requested a review from nazar-pc January 6, 2025 09:45
@@ -28,7 +28,7 @@ use tracing::{debug, trace};
// This code was copied and modified from subspace_service::sync_from_dsn::download_and_reconstruct_blocks():
// <https://github.com/autonomys/subspace/blob/d71ca47e45e1b53cd2e472413caa23472a91cd74/crates/subspace-service/src/sync_from_dsn/import_blocks.rs#L236-L322>
pub async fn download_pieces<PG>(
piece_indexes: Vec<PieceIndex>,
piece_indexes: &Vec<PieceIndex>,
Copy link
Member

Choose a reason for hiding this comment

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

Should be &[PieceIndex] in such cases

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah I tried that and something weird happened with type inference. Happy to have another go next time around.

@teor2345 teor2345 added this pull request to the merge queue Jan 6, 2025
Merged via the queue into main with commit f9b1769 Jan 6, 2025
8 checks passed
@teor2345 teor2345 deleted the gateway-string-or-int branch January 6, 2025 23:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-node This PR introduces breaking changes to the node implementation improvement it is already working, but can be better
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Take BlockNumber as a JSON number in subspace gateway HTTP
3 participants