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

fix chunk fetching network compatibility zombienet test #6988

Merged
merged 3 commits into from
Jan 6, 2025

Conversation

alindima
Copy link
Contributor

Fix this zombienet test

It was failing because in #6452 I enabled the v2 receipts for testnet genesis,
so the collators started sending v2 receipts with zeroed collator signatures to old validators that were still checking those signatures (which lead to disputes, since new validators considered the candidates valid).

The fix is to also use an old image for collators, so that we don't create v2 receipts.

We cannot remove this test yet because collators also perform chunk recovery, so until all collators are upgraded, we need to maintain this compatibility with the old protocol version (which is also why systematic recovery was not yet enabled)

@alindima alindima added R0-silent Changes should not be mentioned in any release notes T10-tests This PR/Issue is related to tests. labels Dec 23, 2024
Copy link
Contributor

@AndreiEres AndreiEres left a comment

Choose a reason for hiding this comment

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

Shouldn't we add any todo here?

@alindima
Copy link
Contributor Author

alindima commented Jan 6, 2025

Shouldn't we add any todo here?

What do you mean @AndreiEres ?

@AndreiEres
Copy link
Contributor

Shouldn't we add any todo here?

What do you mean @AndreiEres ?

Hardcoding a debug image doesn't seem appropriate. I assume it's just a temporary solution, right?

@alindima
Copy link
Contributor Author

alindima commented Jan 6, 2025

Shouldn't we add any todo here?

What do you mean @AndreiEres ?

Hardcoding a debug image doesn't seem appropriate. I assume it's just a temporary solution, right?

Why do you think it's not appropriate? I can't think of a better solution

We already have multiple tests doing this and as far as I've seen, these debug images are never deleted.
Tests that do this will eventually be deleted once we no longer care about backwards compatibility with these versions (once all nodes have been upgraded).

@alindima alindima added this pull request to the merge queue Jan 6, 2025
Merged via the queue into master with commit ffa90d0 Jan 6, 2025
200 of 202 checks passed
@alindima alindima deleted the alindima/fix-0014-test branch January 6, 2025 10:32
ordian added a commit that referenced this pull request Jan 7, 2025
* master: (256 commits)
  fix chunk fetching network compatibility zombienet test (#6988)
  chore: delete repeat words (#7034)
  Print taplo version in CI (#7041)
  Implement cumulus StorageWeightReclaim as wrapping transaction extension + frame system ReclaimWeight (#6140)
  Make `TransactionExtension` tuple of tuple transparent for implication (#7028)
  Replace duplicated whitelist with whitelisted_storage_keys (#7024)
  [WIP] Fix networking-benchmarks (#7036)
  [docs] Fix release naming (#7032)
  migrate pallet-mixnet to umbrella crate (#6986)
  Improve remote externalities logging (#7021)
  Fix polkadot sdk doc. (#7022)
  Remove warning log from frame-omni-bencher CLI (#7020)
  [pallet-revive] fix file case (#6981)
  Add workflow for networking benchmarks (#7029)
  [CI] Skip SemVer on R0-silent and update docs (#6285)
  correct path in cumulus README (#7001)
  sync: Send already connected peers to new subscribers (#7011)
  Excluding chainlink domain for link checker CI (#6524)
  pallet-bounties: Fix benchmarks for 0 ED (#7013)
  Log peerset set ID -> protocol name mapping (#7005)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
R0-silent Changes should not be mentioned in any release notes T10-tests This PR/Issue is related to tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants