Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Fix fungible and fungibles set_balance return value #13851

Merged
merged 2 commits into from
Apr 11, 2023

Conversation

liamaharon
Copy link
Contributor

@liamaharon liamaharon commented Apr 7, 2023

There is a bug in the calculation of the return value of the fungible and fungibles set_balance default implementation.

The doc comment says it should return the new balance, but instead it returns requested_balance +/- balance_change.

I have tests in this PR that show the incorrect return value.

This test

cargo test --package pallet-balances -- tests::fungible_conformance_tests::set_balance_mint_success --nocapture

shows

initial_balance: 11
set_balance_amount: 16
expected_set_balance_return_value: 16
actual_set_balance_return_value: 21

and this test

cargo test --package pallet-balances -- tests::fungible_conformance_tests::set_balance_burn_success --nocapture

shows

initial_balance: 11
set_balance_amount: 6
expected_set_balance_return_value: 6
actual_set_balance_return_value: 1

Luckily (and I guess why this was not caught earlier), I couldn't find anywhere in Substrate or Polkadot that the return value for set_balance is used, so unless there is concern about ecosystem projects relying on the incorrect behavior I think this is safe to merge.

@liamaharon liamaharon added A0-please_review Pull request needs code review. C1-low PR touches the given topic and has a low impact on builders. B1-note_worthy Changes should be noted in the release notes T1-runtime This PR/Issue is related to the topic “runtime”. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit labels Apr 7, 2023
@liamaharon liamaharon merged commit a1f71b8 into master Apr 11, 2023
@liamaharon liamaharon deleted the liam-fix-set-balance-return-value branch April 11, 2023 14:17
@Polkadot-Forum
Copy link

This pull request has been mentioned on Polkadot Forum. There might be relevant details there:

https://forum.polkadot.network/t/april-updates-for-substrate-and-polkadot-devs/2764/1

nathanwhit pushed a commit to nathanwhit/substrate that referenced this pull request Jul 19, 2023
* Fix fungible set_balance return value

* fix fungibles set_balance return value
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B1-note_worthy Changes should be noted in the release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit T1-runtime This PR/Issue is related to the topic “runtime”.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants