add pallet::call to hotfix sufficients for non-zero nonce accounts #619
Conversation
nbaztec
force-pushed
the
nish-upstream-hotfix
branch
from
6ca3149 to
fc65741
Compare
frame/evm/src/lib.rs
Outdated
| #[pallet::weight( | ||
| <T as pallet::Config>::WeightInfo::hotfix_inc_account_sufficients(addresses.len().try_into().unwrap_or(u32::MAX)) | ||
| )] | ||
| pub fn hotfix_inc_account_sufficients( |
There was a problem hiding this comment.
Can you put this under a feature flag?
There was a problem hiding this comment.
I tried it out but seems the generated code (pallet::call procedural macro) still contains references to this code even though the function body is omitted - thereby breaking compilation. Are you aware of a better way to put it behind a feature flag?
There was a problem hiding this comment.
Seems it's discouraged to conditional compile pallet calls since that can break an API quite easily. We could conditionally compile and return an unimplemented error on invocation, but I'm unsure how useful that might be.
There was a problem hiding this comment.
Sorry to jump in like this:
But should this not rather be a storage migration instead of an extrinsic?
There was a problem hiding this comment.
Yeah if you don't have too many accounts in the state already, a migration will be preferred. Alternatively, I believe you just need this call in one single runtime upgrade (which you manually call all addresses yourself), and then it can be removed. Given that this was a bug in Frontier that probably some other chains will also need it for fixing, we may include that in a Frontier release. We'd need to prepare a changelog and announce it though.
There was a problem hiding this comment.
Shall I add a basic version of a CHANGELOG.md ?
|
Isn't it possible to have the pallet separate from the existing one, so that parachain needing it could include it in their runtime, but it wouldn't mess default pallets ? |
|
Please change this to either a migration script or to put the hotfix in a standalone pallet. |
frame/hotfix-sufficients/src/lib.rs
Outdated
| let refs = frame_system::Pallet::<T>::consumers(&account_id) | ||
| + frame_system::Pallet::<T>::providers(&account_id) | ||
| + frame_system::Pallet::<T>::sufficients(&account_id); |
There was a problem hiding this comment.
We might want to use safe math here
(That amount of references would be absurd, but better safe than sorry :)
…olkadot-evm#619) * add pallet::call to hotfix sufficients for non-zero nonce accounts * make hotfix to fix sufficients payable * cargo fmt * use distinct pallet for hotfix * cleanup traces from pallet evm * add pallet-hotfix-sufficients * fix description * fix benchmarking * update test condition, add docs * use safe add Co-authored-by: tgmichel <telmo@purestake.com>
HotfixSufficientswith callhotfix_inc_account_sufficientsto patch existing accounts with a non-zero nonce but a zerosufficientsvalue. The value needs to be non-zero to be consistent with our current runtime logic.$ ./scripts/benchmark.sh 1) pallet_evm, runner_execute 2) pallet_evm, hotfix_inc_account_sufficients 3) EXIT #? 2The signed extrinsic call can be called by any account.