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

Remove deprecated pallet_balances's set_balance_deprecated and transfer dispatchables #1226

Merged
merged 13 commits into from
Sep 6, 2023

Conversation

juangirini
Copy link
Contributor

@juangirini juangirini commented Aug 28, 2023

Fixes #147
Fixes #148

The pallet_balances dispatchables set_balance_deprecated and transfer were deprecated in paritytech/substrate#12951 and have now been removed.
Instead force_set_balance and transfer_allow_death should be used respectively.

@juangirini juangirini self-assigned this Aug 29, 2023
@juangirini juangirini added the T2-pallets This PR/Issue is related to a particular pallet. label Aug 29, 2023
@juangirini juangirini requested a review from a team August 29, 2023 07:45
@juangirini juangirini marked this pull request as ready for review August 29, 2023 07:46
@paritytech-ci paritytech-ci requested review from a team August 29, 2023 07:47
@juangirini juangirini changed the title Remove deprecated dispatchables Remove deprecated pallet_balances's set_balance_deprecated and transfer dispatchables Aug 29, 2023
@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: test-linux-stable 3/3
Logs: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/3474324

@liamaharon
Copy link
Contributor

Have we done an impact assessment on the removal of transfer? I have a feeling it could break a lot of front-ends.

@juangirini
Copy link
Contributor Author

There's no assessment. And yes, there might be some broken frontends if their code hasn't been updated since these dispatchables have been marked as deprecated 6 months ago.
We don't have a proactive way to let the implementors know about these kind of deprecations, there is an open discussion about it #182 (comment)

@juangirini juangirini removed request for a team August 30, 2023 07:41
dest: AccountIdLookupOf<T>,
#[pallet::compact] value: T::Balance,
) -> DispatchResult {
let source = ensure_signed(origin)?;
Copy link
Member

Choose a reason for hiding this comment

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

The only thing that we could do is to always return an error with a message that the other call should be used.
But its unclean i think…

Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like we missed an opportunity to mark this function as #[deprecated]. Maybe this would've clued people in that this extrinsic is going away...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There was a deprecation message, tagging #[deprecated] wouldn't have helped much here anyway as external calls don't get notified of it. See #182 (comment)

@ggwpez ggwpez added the T8-polkadot This PR/Issue is related to/affects the Polkadot network. label Aug 30, 2023
@ggwpez
Copy link
Member

ggwpez commented Aug 30, 2023

This definitely needs to go into the change log. Lets see if that works out with the labels.

@juangirini
Copy link
Contributor Author

@jacogr to keep you in the loop that this change is comming

@command-bot
Copy link

command-bot bot commented Sep 6, 2023

@juangirini Command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh" has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/3588093 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/3588093/artifacts/download.

@juangirini juangirini enabled auto-merge (squash) September 6, 2023 12:29
@juangirini juangirini merged commit 68ab943 into master Sep 6, 2023
@juangirini juangirini deleted the jg/deprecate-set-balance-deprecated branch September 6, 2023 13:06
ordian added a commit that referenced this pull request Sep 6, 2023
* master: (24 commits)
  GHW for building and publishing docker images (#1391)
  pallet asset-conversion additional quote tests (#1371)
  Remove deprecated `pallet_balances`'s `set_balance_deprecated` and `transfer` dispatchables (#1226)
  Fix PRdoc check (#1419)
  Fix the wasm runtime substitute caching bug (#1416)
  Bump enumn from 0.1.11 to 0.1.12 (#1412)
  RFC 14: Improve locking mechanism for parachains (#1290)
  Add PRdoc check (#1408)
  fmt fixes (#1413)
  Enforce a decoding limit in MultiAssets (#1395)
  Remove dynamic dispatch using `Ext` (#1399)
  Remove redundant calls to `borrow()` (#1393)
  Get rid of polling in `WarpSync` (#1265)
  Bump actions/checkout from 3 to 4 (#1398)
  Bump thiserror from 1.0.47 to 1.0.48 (#1396)
  Move Relay-Specific Shared Code to One Place (#1193)
  rust docs: add simple analytics (#1377)
  Contracts: Update read_sandbox (#1390)
  Extract block announce validation from `ChainSync` (#1170)
  [ci] Remove runtime-benchmarks from tests (#1335)
  ...
@xlc
Copy link
Contributor

xlc commented Sep 6, 2023

Is this a transction_version breaking change?
Is this going to impact existing Ledger app?

@juangirini
Copy link
Contributor Author

Is this a transction_version breaking change? Is this going to impact existing Ledger app?

Yes, it is a transaction_version breaking change. And yes, it is going to impact existing apps that haven't yet updated to use the new dispatchables after the release of the Currency deprecation PR paritytech/substrate#12951
The recommendation is to migrate to the new force_set_balance and transfer_allow_death ASAP

ordian added a commit that referenced this pull request Sep 7, 2023
* master: (28 commits)
  Adds base benchmark for do_tick in broker pallet (#1235)
  zombienet: use another collator image for the slashing test (#1386)
  Prevent a fail prdoc check to block (#1433)
  Fix nothing scheduled on session boundary (#1403)
  GHW for building and publishing docker images (#1391)
  pallet asset-conversion additional quote tests (#1371)
  Remove deprecated `pallet_balances`'s `set_balance_deprecated` and `transfer` dispatchables (#1226)
  Fix PRdoc check (#1419)
  Fix the wasm runtime substitute caching bug (#1416)
  Bump enumn from 0.1.11 to 0.1.12 (#1412)
  RFC 14: Improve locking mechanism for parachains (#1290)
  Add PRdoc check (#1408)
  fmt fixes (#1413)
  Enforce a decoding limit in MultiAssets (#1395)
  Remove dynamic dispatch using `Ext` (#1399)
  Remove redundant calls to `borrow()` (#1393)
  Get rid of polling in `WarpSync` (#1265)
  Bump actions/checkout from 3 to 4 (#1398)
  Bump thiserror from 1.0.47 to 1.0.48 (#1396)
  Move Relay-Specific Shared Code to One Place (#1193)
  ...
Ank4n pushed a commit that referenced this pull request Sep 8, 2023
…ransfer` dispatchables (#1226)

* remove deprecated dispatchables

* update test

* update tests

* update tests

* add prdocs

* add prdoc

* Update docs/prdoc/pr_1226.prdoc

Co-authored-by: Chevdor <chevdor@users.noreply.github.com>

* move prdoc file

---------

Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>
Co-authored-by: Chevdor <chevdor@users.noreply.github.com>
Daanvdplas pushed a commit that referenced this pull request Sep 11, 2023
…ransfer` dispatchables (#1226)

* remove deprecated dispatchables

* update test

* update tests

* update tests

* add prdocs

* add prdoc

* Update docs/prdoc/pr_1226.prdoc

Co-authored-by: Chevdor <chevdor@users.noreply.github.com>

* move prdoc file

---------

Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>
Co-authored-by: Chevdor <chevdor@users.noreply.github.com>
mordamax added a commit to paritytech/polkadot-testnet-faucet that referenced this pull request Oct 17, 2023
transfer is undefined now for some reason (reason ->
paritytech/polkadot-sdk#1226)
replaced with trasferKeepAlive

<img width="836" alt="image"
src="https://github.com/paritytech/polkadot-testnet-faucet/assets/1177472/471be755-8160-45d6-82be-2fc3e2f6f080">


The reason is valid, as `transfer` was actually `transfer_allow_death`,
while we supposed to use transfer_keep_alive from the start
HCastano added a commit to entropyxyz/entropy-core that referenced this pull request Jan 19, 2024
This includes the deprecation of `Balances::transfer` from
paritytech/polkadot-sdk#1226.
HCastano added a commit to entropyxyz/entropy-core that referenced this pull request Jan 23, 2024
* Get `entropy-shared` compiling with updated deps

* Fix `entropy-shared` build for `wasm-no-std`

* Make `pallet-staking-extension` compile

Starts introducing some of the changes brought in by
paritytech/polkadot-sdk#1484.

* Introduce staking changes from paritytech/substrate#12970

* Add RuntimeFreezeReason from paritytech/polkadot-sdk#1900

* Re-map errors in staking extension pallet

* Bump `pallet-programs`

* Missed a crate dep for staking extension

* Remove commented out code

* Get `pallet-relayer` compiling

* Fix relayer test compilation

* Fix `pallet-free-tx`'s tests

* Reorder config types

* Bump `pallet-propagation` dependencies

* Bump `pallet-slashing` dependencies

* Bump `pallet-transaction-pause`

This includes the deprecation of `Balances::transfer` from
paritytech/polkadot-sdk#1226.

* TaploFmt

* Use `StakingAccount` instead of `.into()`

* Import `vec` macro in `pallet-slashing`

* Import `vec` in other pallets

* Bump TOML dependencies for runtime

Haven't fixed the Rust code yet though

* Self define types instead of using `node-primitives`

* Update staking related configs

Values were mostly taken from the Polkadot runtime config

* Update `pallet-preimage` config

Changes introduced in paritytech/polkadot-sdk#1363

* Add `MaxNominators` to `pallet_babe`

Introduced in paritytech/substrate#14471

* Add `MaxNominators` to `pallet_grandpa`

Also introduced in paritytech/substrate#14471

* Add `RuntimeFreezeReason` to `pallet_nomination_pools`

* Add `MaxTipAmount` to `pallet_tips`

Introduced in paritytech/polkadot-sdk#1709

* Add `IdentityInformation` to `pallet_identity`

Introduced in paritytech/polkadot-sdk#1661

* finish runtime

* Use `crates.io` packages in `entropy` TOML

This doesn't compile yet

* Remove reference to `node_primitives`

* Move imports under correct dependency section

These shouldn't be built-deps, my bad

* Change `WarpSyncParams` import to come from `sc_network_sync`

Change introduced here: paritytech/polkadot-sdk#1912

* Add missing `sc-consensus-babe` package

* Remove second generic from `DefaultImportQueue`

This was removed in paritytech/substrate#14612

* Add temporary cursed RPC bounds

* Switch `sp-consensus-grandpa` to be from Crates

* Add Grandpa justification period to service

* Rename `justification_period to` `justification_generation_period`

* Add missing `block_relay` param

Introduced in paritytech/polkadot-sdk#1524

* Try to clean up runtime types

* Add `opaque` types to runtime

These match what was in `node-primitives`. In particular we're interested in the `Block` type which
we were using in a lot of places, and I incorrectly changed to be `Unchechecked` instead of
`Opaque`.

* Use opaque block type in service

* Add full session key impls

* Remove cursed trait bounds for RPC

* Convert some tabs to spaces

Sorry, I had to

* Import `Vec` from `sp_std` in benchmarks

Turns out the issue responsible for this change is this:
paritytech/polkadot-sdk#172

* Pull `TrackedStorageKey` from `sp_storage`

From here: paritytech/substrate#14787

* Add `BenchmarkHelper` to treasury pallet

* TaploFmt

* change max freezes

* Sort TOML imports in runtime manifest

* Move `version` field to be first in runtime manifest

* Organize imports in pallet manifests

* Sort and organize imports for client

* Address TODOs in `entropy-shared`

* Remove TODOs related to `node-primitives`

* Pull `substrate-wasm-builder` from crates.io

* TaploFmt

* Fix benchmarking import post-merge

* Fix another program import port-merge

* Import `Vec` to programs benches

* Wrong vec import 🙃

* Poke CI

* Bump node metadata

---------

Co-authored-by: Jesse Abramowitz <jesse@entropy.xyz>
bgallois pushed a commit to duniter/duniter-polkadot-sdk that referenced this pull request Mar 25, 2024
…ransfer` dispatchables (paritytech#1226)

* remove deprecated dispatchables

* update test

* update tests

* update tests

* add prdocs

* add prdoc

* Update docs/prdoc/pr_1226.prdoc

Co-authored-by: Chevdor <chevdor@users.noreply.github.com>

* move prdoc file

---------

Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>
Co-authored-by: Chevdor <chevdor@users.noreply.github.com>
bkchr pushed a commit that referenced this pull request Apr 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T2-pallets This PR/Issue is related to a particular pallet. T8-polkadot This PR/Issue is related to/affects the Polkadot network.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[Deprecation] Remove deprecated Balances::transfer [Deprecation] Remove Balances::set_balance_deprecated
10 participants