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

Contracts: migration v15 acc dec_consumers check #1696

Closed
wants to merge 1 commit into from
Closed

Conversation

pgherveou
Copy link
Contributor

@pgherveou pgherveou commented Sep 25, 2023

Current migration step could produce error logs
Logic error: Unexpected underflow in reducing consumer

Tested with

 RUST_LOG="runtime::contracts=info,try-runtime::cli=debug" try-polkadot-parachain try-runtime \
 --runtime target/release/wbuild/contracts-rococo-runtime/contracts_rococo_runtime.wasm \
 --chain contracts-rococo \
 on-runtime-upgrade snap --snapshot-path ./contracts-rococo-9420@latest.snap

Patch generated with

RUST_LOG="try-runtime::cli=debug" try-polkadot-parachain try-runtime \
 --runtime existing \
 --chain contracts-rococo \
create-snapshot \
--uri wss://rococo-contracts-rpc.polkadot.io:443

@pgherveou pgherveou requested a review from athei as a code owner September 25, 2023 11:56
@pgherveou pgherveou requested a review from a team September 25, 2023 11:56
@pgherveou pgherveou added T2-pallets This PR/Issue is related to a particular pallet. R0-silent Changes should not be mentioned in any release notes labels Sep 27, 2023
@pgherveou
Copy link
Contributor Author

pgherveou commented Sep 29, 2023

@juangirini any idea why the consumer refcount could be 0 pre-migration in some cases?

Screenshot 2023-09-25 at 14 06 01

Copy link
Contributor

@juangirini juangirini left a comment

Choose a reason for hiding this comment

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

Please see paritytech/substrate#14589 (comment), we don't need such a check. If we can't decrease the consumers then there is an error and it is ok to print it. Anyway the flow continues and the rest executes correctly, the only thing that it is done with the conditional is to hide the error.

@juangirini any idea why the consumer refcount could be 0 pre-migration in some cases?

The error was introduced on v10 when the deposit account was created without incrementing the consumers. As the deposits accounts don't exist anymore it is ok not to fix that v10 bug as is not relevant anymore.

We should close this PR

@pgherveou
Copy link
Contributor Author

Thanks Juan, maybe we should add a comment on the code to explain what you just said. Afk but will update later

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 T2-pallets This PR/Issue is related to a particular pallet.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants