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 panic in case of invalid checkpoint in chain spec #603

Merged
merged 6 commits into from
May 25, 2023

Conversation

tomaka
Copy link
Contributor

@tomaka tomaka commented May 25, 2023

Fix paritytech/substrate#598

cc @lexnv

Would be great to have a test case, but generating one myself would legitimately take me several hours.
@lexnv If you could upload somewhere the chain spec JSON that triggers a panic so that I include it as a regression test in this PR, it would be great 🙏 If not possible or too annoying, then it's not a big deal.

@lexnv
Copy link
Contributor

lexnv commented May 25, 2023

Hi,

This is the spec that I was using:
https://gist.github.com/lexnv/9050f533d8ec0bc5c31b897b007508da

Before this patch

thread 'main' panicked at 'attempt to subtract with overflow', /Users/lexnv/.cargo/git/checkouts/smoldot-2dcbf637e11a77d4/43b3306/lib/src/chain_spec.rs:445:37
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

After this patch

Error: LightClient(AddChainError("Invalid checkpoint in chain specification: EmptyBabeEpochsList"))

lexnv
lexnv previously approved these changes May 25, 2023
@tomaka
Copy link
Contributor Author

tomaka commented May 25, 2023

Error: LightClient(AddChainError("Invalid checkpoint in chain specification: EmptyBabeEpochsList"))

Ah, thanks for testing! That's very weird and annoying, as to me this indicates an invalid checkpoint.

@tomaka
Copy link
Contributor Author

tomaka commented May 25, 2023

The core of the problem is that I have no idea what constitutes a valid checkpoint, cc paritytech/polkadot-sdk#60

I suppose that if the list of epochs is empty, then we should simply ignore the checkpoint.

@tomaka tomaka added this pull request to the merge queue May 25, 2023
Merged via the queue into smol-dot:main with commit 8f6e978 May 25, 2023
@tomaka tomaka deleted the fix-598 branch May 25, 2023 11:23
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.

2 participants