Skip to content

tests: Add zombienet test for bitswap_v1_get#3240

Merged
BigTava merged 12 commits intomainfrom
tiago-e2e-test-bitswap-get
May 7, 2026
Merged

tests: Add zombienet test for bitswap_v1_get#3240
BigTava merged 12 commits intomainfrom
tiago-e2e-test-bitswap-get

Conversation

@BigTava
Copy link
Copy Markdown
Contributor

@BigTava BigTava commented Apr 30, 2026

Closes #3232

@BigTava BigTava marked this pull request as draft April 30, 2026 16:56
@BigTava BigTava marked this pull request as ready for review April 30, 2026 16:56
@BigTava
Copy link
Copy Markdown
Contributor Author

BigTava commented Apr 30, 2026

@skunert you mention snapshots should live on Google Drive, but I haven't seen anything similar. polkadot-bulletin-chain stores its build outputs as GitHub artifacts (e.g. release.yml#L98-L103). Should we do that instead?

@BigTava BigTava changed the title e2e-tests: Add zombienet test for bitswap_v1_get tests: Add zombienet test for bitswap_v1_get Apr 30, 2026
@skunert
Copy link
Copy Markdown
Contributor

skunert commented May 4, 2026

@BigTava An example can be seen here: https://github.com/paritytech/polkadot-sdk/blob/3501f2c3c8e46d0b726994065a05615995f03880/cumulus/zombienet/zombienet-sdk/tests/zombie_ci/full_node_warp_sync/common.rs#L26

I think artifacts can be of limited size only and have a retention period. These snapshots we should only generate very rarely and then always reuse.

@skunert
Copy link
Copy Markdown
Contributor

skunert commented May 4, 2026

Hmm looks like I lost access to our cloud storage, @pepoviola what is the current way to store these snapshots, do you still have access?

@pepoviola
Copy link
Copy Markdown

Hmm looks like I lost access to our cloud storage, @pepoviola what is the current way to store these snapshots, do you still have access?

You should have access to zombienet-db-snaps bucket, let me know if doesn't works for you and I can re-check.
Thx!

BigTava added 3 commits May 4, 2026 17:23
Snapshots live in the zombienet-db-snaps bucket under
smoldot/bulletin_fetch/, accessed via hardcoded URLs that
with_db_snapshot() downloads automatically. Local override is wired
through DB_SNAPSHOT_*_OVERRIDE env vars.

Drops the manifest.json read on the fetch path: payload metadata is
derived from bulletin::payloads() at runtime instead.
Renamed to bulletin::payloads() in the code earlier; README missed it.
@BigTava
Copy link
Copy Markdown
Contributor Author

BigTava commented May 4, 2026

An example can be seen here

Thanks @skunert! I added the remaining code based on this example. The snapshots come out to ~50 MB total. Someone with bucket-write access needs to upload them for the CI job to pass.

Copy link
Copy Markdown
Contributor

@lrubasze lrubasze left a comment

Choose a reason for hiding this comment

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

Nice work!
Few comments from my side

Comment thread e2e-tests/js/bulletin_fetch.js Outdated
Comment thread e2e-tests/src/bulletin.rs Outdated
Comment thread e2e-tests/tests/bulletin_fetch.rs Outdated
Comment thread e2e-tests/js/bulletin_fetch.js
Comment thread e2e-tests/js/bulletin_fetch.js Outdated
BigTava and others added 5 commits May 5, 2026 17:30
The DB_SNAPSHOT_* constants now point at -2026-05-04.tgz so the bucket
keeps a clear version history instead of silently overwriting on each
refresh. Bump the date in the constants (and re-upload) whenever the
bulletin runtime or bulletin::payloads() changes.
- Mixed-availability scenario now iterates every full-only payload
  instead of only the first match, so both 31 B and 1 MiB cases
  exercise gossip fallback.
- Rename payload labels to `all-nodes-*` / `one-node-*` so they
  describe corpus location instead of opaque `both-*` / `full-only-*`.
- Hoist `RELAY_CHAIN`, `RELAY_BINARY`, `PARA_BINARY` into
  `bulletin.rs`; both test files reference the shared consts.
- Trim the `bitswapGetWithRetry` doc to a single line.
- `cargo fmt`.
The bare `sendRpcAndWait` was relying on the implicit ordering that
prior `known-*` scenarios had already warmed up smoldot. Wrap this
call too so the assertion is robust to test reordering and cold-start
hangs. `-32602` propagates on the first attempt anyway, so behaviour
in the happy path is unchanged.
@michalkucharczyk
Copy link
Copy Markdown
Contributor

Dumb question: does e2e-tests/tests/bulletin_generate_snapshot.rs really belongs to smoldot repo domain?

I can imagine such snapshot can be used for bulletin related functionality also in other repos.
And one more thought - should snapshot generation be part of zombienet sdk? Again I can imagine this code will be duplicated in every test that requires snapshot (cc: @pepoviola).

@lrubasze @skunert - does it feel right for you?
@karolk91 wdyt?

@lrubasze
Copy link
Copy Markdown
Contributor

lrubasze commented May 7, 2026

Dumb question: does e2e-tests/tests/bulletin_generate_snapshot.rs really belongs to smoldot repo domain?

I can imagine such snapshot can be used for bulletin related functionality also in other repos. And one more thought - should snapshot generation be part of zombienet sdk? Again I can imagine this code will be duplicated in every test that requires snapshot (cc: @pepoviola).

@lrubasze @skunert - does it feel right for you? @karolk91 wdyt?

I think this is fine.
We use similar approach in full_node_warp_sync test

Tests that use snapshot and test that generates it need to have the same network setup. So i think it is good to keep them in one place. Unless we outsource zombienet network setup too.

@skunert
Copy link
Copy Markdown
Contributor

skunert commented May 7, 2026

should snapshot generation be part of zombienet sdk?

I thought about this in the past too, my 2c is that its okay to include this per-repo, because sometimes the snapshots need to replicate specific scenarios. For bulletin I can imagine scenarios with specific counts of renew/store tx etc.

@BigTava
Copy link
Copy Markdown
Contributor Author

BigTava commented May 7, 2026

@lrubasze @skunert needs 2 approvals. Thanks

@BigTava BigTava requested a review from lrubasze May 7, 2026 10:11
Copy link
Copy Markdown
Contributor

@skunert skunert left a comment

Choose a reason for hiding this comment

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

Nice!

Comment thread .github/workflows/zombienet.yml Outdated
);
}

for (const payload of payloads.filter((p) => !p.on_partial)) {
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.

One nit (don't think we need to change it right now). Would be nice to know if we actually hit the partial case where a CID is unavailable. Imagine if peer switching was broken and smoldot would always ask the same peer, then this test would become flaky depending on which peer is chosen for request right? Not sure we have a good way to handle this case currently.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Smoldot handles DontHave silently in bitswap_service.rs, so there was nothing for the test to scrape today. Feel free to file that as a follow-up issue to discuss and implement.

Avoids matrix-slot collision with PR #3242 (more smoke tests), which
takes 0004-0006.
@BigTava BigTava merged commit d4b786c into main May 7, 2026
33 checks passed
@BigTava BigTava deleted the tiago-e2e-test-bitswap-get branch May 7, 2026 11:58
@pepoviola
Copy link
Copy Markdown

should snapshot generation be part of zombienet sdk?

I thought about this in the past too, my 2c is that its okay to include this per-repo, because sometimes the snapshots need to replicate specific scenarios. For bulletin I can imagine scenarios with specific counts of renew/store tx etc.

I aggre, but we can offload the creation logic to zombienet-sdk itself, so per repo you only need to setup the network and call something like network.generate_snap(). I will add a new issue to track this in zombienet repo.

Thx!

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.

Zombienet for bitswap_v1_get

5 participants