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

Collectives: teleport slashed assets - #1433

Merged
joepetrowski merged 11 commits into
joe-alliancefrom
collectives-slash-assets-handler
Jul 21, 2022
Merged

joepetrowski merged 11 commits into
joe-alliancefrom
collectives-slash-assets-handler

Conversation

@muharem

@muharem muharem commented Jul 7, 2022 •

Copy link
Copy Markdown
Contributor

Teleport slashed assets to the Polkadot treasury account.

Passes integration tests paritytech/parachains-integration-tests#26

@muharem muharem added A0-pleasereview B0-silent Changes should not be mentioned in any release notes labels Jul 7, 2022
@joepetrowski joepetrowski mentioned this pull request Jul 7, 2022
5 tasks done
Comment thread Cargo.lock Outdated
@xlc

xlc commented Jul 7, 2022

Copy link
Copy Markdown
Contributor

This sounds like a bad idea that makes DoS relaychain much easier. Why not just sent to local treasury and have another method to teleport local treasury to parent treasury.

Comment thread parachains/runtimes/collectives/collectives-polkadot/src/constants.rs Outdated

@gilescope gilescope left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice work!

Comment thread parachains/runtimes/collectives/collectives-polkadot/src/impls.rs Outdated
@joepetrowski

Copy link
Copy Markdown
Contributor

This sounds like a bad idea that makes DoS relaychain much easier.

How? The DoS protection is the fact that someone is getting slashed. And the slasher in this case is the Polkadot Alliance, which is semi-trusted. If the Alliance is DoSing the network then we have bigger problems.

Why not just sent to local treasury and have another method to teleport local treasury to parent treasury.

  1. Because there is no local treasury.
  2. The Alliance members should not be in control of the tokens.
  3. There should be a clear story about what happens when someone is slashed and where the tokens go. Putting them in limbo just adds confusion about the consequences of getting slashed and more work for others to sort it out later.

@xlc

xlc commented Jul 13, 2022

Copy link
Copy Markdown
Contributor

I guess if slash operation is protected it will be fine. Still, having fun trying to figure out the correct weight for extrinsic that triggers slash.

By local treasury I mean just a local pallet account that is only accessible by root. Then relaychain governance can use XCM to teleport those funds back if needed. There is no need to have a treasury pallet.

The slash operation and amount is very clearly defined so I guess current approach is fine. However image if somehow resulting the slash amount to be less than ED, the cost of sending XCM is just not worth the teleport. This approach is unsafe as a generic way to handle slash. My only worry is in future, some new introduction of the slash mechanism could result unexpected complications.

@joepetrowski

Copy link
Copy Markdown
Contributor

The slash operation and amount is very clearly defined so I guess current approach is fine. However image if somehow resulting the slash amount to be less than ED, the cost of sending XCM is just not worth the teleport. This approach is unsafe as a generic way to handle slash. My only worry is in future, some new introduction of the slash mechanism could result unexpected complications.

I don't think this is a huge concern. If slashes are less than the ED then they are not really that strong of a disincentive.

Comment thread parachains/runtimes/collectives/collectives-polkadot/src/lib.rs Outdated
Comment thread parachains/runtimes/collectives/collectives-polkadot/src/lib.rs Outdated
@muharem

muharem commented Jul 21, 2022

Copy link
Copy Markdown
Contributor Author

Integration tests for the use case - paritytech/parachains-integration-tests#26
PR to allow teleport to the westend relay chain - paritytech/parachains-integration-tests#26

Comment thread parachains/runtimes/collectives/collectives-polkadot/src/impls.rs Outdated
@muharem
muharem requested review from KiChjang and joepetrowski July 21, 2022 13:16
@muharem
muharem requested a review from NachoPal July 21, 2022 13:16
Comment thread parachains/runtimes/collectives/collectives-polkadot/src/constants.rs Outdated
@joepetrowski
joepetrowski merged commit 6087ae8 into joe-alliance Jul 21, 2022
@joepetrowski
joepetrowski deleted the collectives-slash-assets-handler branch July 21, 2022 14:47
paritytech-processbot Bot pushed a commit that referenced this pull request Aug 11, 2022
* add new runtime and remove unnecessary pallets

* make runtime build

* add collectives to collator node

* sketch alliance config in runtime

* Slash handler was supposed to be commented out (for now)

* correct signature

* move to impls

* add alliance to runtime

* rustfmt

* IsReserve, remove Ping, update fn deposit

* add transaction_payment event

* Update parachains/runtimes/collectives/collectives-polkadot/src/lib.rs

Co-authored-by: Squirrel <gilescope@gmail.com>

* fmt

* add genesis config to chain spec

* fix merge

* local and dev configs only (for now)

* remove duplicate imports

* Collectives polkadot runtime to cargo workspace members (#1397)

* Collectives polkadot runtime: use unit type impl for identity verifier (#1398)

* apply fn rename

* fmt

* one less todo

* Less code in magic macros (#1407)

* Less code in magic macros

* cargo fmt

* Bench alliance (#1427)

* add benchmarks

* call one script from the other

* shebang changes so works on nixos too.

* bench in parallel as separate jobs

* hyphens can turn into underscores

* remove workaround to trigger bench

Co-authored-by: alvicsam <alvicsam@gmail.com>
Co-authored-by: paritytech-ci <paritytech-ci@parity.io>

* enable ci jobs

* fix publish bench results jobs

* chainspecs for collectives-westend (#1441)

* initial chainspecs for collections relay chain

* plumb in the collectives-westend chainspec

* add Runtime::CollectivesWestend

* lock

* Collectives: teleport slashed assets  (#1433)

* Collectives: teleport slashed assets

* fmt

* Cargo.lock > polkadot-parachain 0.9.25

* create temp account for imbalance

* treasury acc id from pallet id

* move accounts into constants, use here junction for assets

* assets location is relay chain, accounts as parameters

* fix typos

* fix typo

* Update parachains/runtimes/collectives/collectives-polkadot/src/constants.rs

Co-authored-by: joe petrowski <25483142+joepetrowski@users.noreply.github.com>

* Move alliance proposal provider to impls.rs (#1464)

* Move to impls alliance proposal provider

* rustfmt

* Bumping spec version

(so that we can redeploy with slashing change.)

* cargo lock

* slurp collectives digest to make appear in release notes (#1473)

* add slurp

* Slurp better :)

* Bring some order

Co-authored-by: Chevdor <chevdor@users.noreply.github.com>
Co-authored-by: Wilfried Kopp <wilfried@parity.io>

* reorder barrier

* Update parachains/runtimes/collectives/collectives-polkadot/src/xcm_config.rs

Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>

* address review

* clean construct runtime

* fmt

* looks pretty but brings in too much

Co-authored-by: Squirrel <gilescope@gmail.com>
Co-authored-by: Muharem Ismailov <ismailov.m.h@gmail.com>
Co-authored-by: alvicsam <alvicsam@gmail.com>
Co-authored-by: paritytech-ci <paritytech-ci@parity.io>
Co-authored-by: Chevdor <chevdor@users.noreply.github.com>
Co-authored-by: Wilfried Kopp <wilfried@parity.io>
Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

B0-silent Changes should not be mentioned in any release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants