Skip to content

Balances Pallet: Emit events when TI is updated in currency impl#4936

Merged
bkchr merged 16 commits into
paritytech:masterfrom
mittal-parth:emit-events-currency
Jul 19, 2024
Merged

Balances Pallet: Emit events when TI is updated in currency impl#4936
bkchr merged 16 commits into
paritytech:masterfrom
mittal-parth:emit-events-currency

Conversation

@mittal-parth

@mittal-parth mittal-parth commented Jul 3, 2024

Copy link
Copy Markdown
Contributor

Description

Previously, in the Currency impl, the implementation of pallet_balances was not emitting any instances of Issued and Rescinded events, even though the Fungible equivalent was.

This PR adds the Issued and Rescinded events in appropriate places in impl_currency along with tests.

Closes #4028

polkadot address: 5GsLutpKjbzsbTphebs9Uy4YK6gTN47MAaz6njPktidjR5cp

@mittal-parth mittal-parth requested a review from a team as a code owner July 3, 2024 13:33
@cla-bot-2021

cla-bot-2021 Bot commented Jul 3, 2024

Copy link
Copy Markdown

User @mittal-parth, please sign the CLA here.

Comment thread substrate/frame/balances/src/impl_currency.rs Outdated
Comment thread prdoc/pr_4936.prdoc Outdated
Comment thread prdoc/pr_4936.prdoc Outdated
@github-actions github-actions Bot requested a review from bkchr July 3, 2024 16:39
@github-actions

github-actions Bot commented Jul 3, 2024

Copy link
Copy Markdown
Contributor

Review required! Latest push from author must always be reviewed

@ggwpez ggwpez added the T2-pallets This PR/Issue is related to a particular pallet. label Jul 3, 2024
@github-actions github-actions Bot requested a review from ggwpez July 3, 2024 17:39
@bkchr bkchr enabled auto-merge July 3, 2024 19:12
auto-merge was automatically disabled July 4, 2024 05:16

Head branch was pushed to by a user without write access

@github-actions github-actions Bot requested a review from bkchr July 4, 2024 05:16
@mittal-parth

Copy link
Copy Markdown
Contributor Author

Sorry for the inconvenience @bkchr 🥲

@bkchr bkchr enabled auto-merge July 4, 2024 05:44
@mittal-parth

Copy link
Copy Markdown
Contributor Author

@bkchr Looks like there is an issue with the full_native_block_import_works test and the sanity check for running Import Benchmarks.

For the first one, there should be the Rescinded event added.

	EventRecord {
		phase: Phase::ApplyExtrinsic(1),
		event: RuntimeEvent::Balances(pallet_balances::Event::Rescinded {
			amount: fees * 2 / 10,
		}),
		topics: vec![],
	},

For the second one, there would 9 events per signed extrinsic because of the extra Issued event.

assert_eq!(
	kitchensink_runtime::System::events().len(),
	(self.block.extrinsics.len() - 2) * 9 + 2,
);

I would like to run these tests locally before pushing just to catch further errors. Is there a way to run them locally? Whenever I run cargo t full_native_block_import_works -- --exact, my system just hangs while building itself.

@bkchr

bkchr commented Jul 4, 2024

Copy link
Copy Markdown
Member

@mittal-parth did you try to just compile staging-node-cli?

cargo test -p staging-node-cli full_native_block_import_works

Otherwise you maybe not have enough main memory? How much do you have?

@mittal-parth

Copy link
Copy Markdown
Contributor Author

@mittal-parth did you try to just compile staging-node-cli?

cargo test -p staging-node-cli full_native_block_import_works

Tried this, same result :(

Otherwise you maybe not have enough main memory? How much do you have?

I have 20GB RAM and 8 GB swap space.

@bkchr

bkchr commented Jul 4, 2024

Copy link
Copy Markdown
Member

Is your system freezing because it runs out of memory or what?

@github-actions github-actions Bot requested review from bkchr and kianenigma July 5, 2024 13:32
@kianenigma

Copy link
Copy Markdown
Contributor

/tip small

@substrate-tip-bot

Copy link
Copy Markdown

@kianenigma A referendum for a small (20 DOT) tip was successfully submitted for @mittal-parth (5GsLutpKjbzsbTphebs9Uy4YK6gTN47MAaz6njPktidjR5cp on polkadot).

Referendum number: 971.
tip

@substrate-tip-bot

Copy link
Copy Markdown

The referendum has appeared on Polkassembly.

@bkchr bkchr enabled auto-merge July 19, 2024 15:05
Comment thread substrate/frame/balances/src/impl_currency.rs
Comment thread substrate/frame/balances/src/impl_currency.rs
@paritytech-cicd-pr

Copy link
Copy Markdown

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

@bkchr bkchr added this pull request to the merge queue Jul 19, 2024
Merged via the queue into paritytech:master with commit 59e3315 Jul 19, 2024
TarekkMA pushed a commit to moonbeam-foundation/polkadot-sdk that referenced this pull request Aug 2, 2024
…itytech#4936)

# Description

Previously, in the `Currency` impl, the implementation of
`pallet_balances` was not emitting any instances of `Issued` and
`Rescinded` events, even though the `Fungible` equivalent was.

This PR adds the `Issued` and `Rescinded` events in appropriate places
in `impl_currency` along with tests.

Closes paritytech#4028 

polkadot address: 5GsLutpKjbzsbTphebs9Uy4YK6gTN47MAaz6njPktidjR5cp

---------

Co-authored-by: Bastian Köcher <git@kchr.de>
Co-authored-by: Bastian Köcher <info@kchr.de>
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.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Emit proper events in Balances when ED is updated

6 participants