Currently `transfer_assets` from pallet-xcm covers 4 main different
transfer types:
- `localReserve`
- `DestinationReserve`
- `Teleport`
- `RemoteReserve`
For the first three, the local execution and the remote message sending
are separated, and fees are deducted in pallet-xcm itself:
https://github.com/paritytech/polkadot-sdk/blob/3410dfb3929462da88be2da813f121d8b1cf46b3/polkadot/xcm/pallet-xcm/src/lib.rs#L1758.
For the 4th case `RemoteReserve`, pallet-xcm is still relying on the
xcm-executor itself to send the message (through the
`initiateReserveWithdraw` instruction). In this case, if delivery fees
need to be charged, it is not possible to do so because the
`jit_withdraw` mode has not being set.
This PR proposes to still use the `initiateReserveWithdraw` but
prepending a `setFeesMode { jit_withdraw: true }` to make sure delivery
fees can be paid.
A test-case is also added to present the aforementioned case
---------
Co-authored-by: Adrian Catangiu <adrian@parity.io>
Bridging fees are calculated using a static ETH/DOT exchange rate that
can deviate significantly from the real-world exchange rate. We
therefore need to add a safety margin to the fee so that users almost
aways cover the cost of relaying.
# FAQ
> Why introduce a `multiplier` parameter instead of configuring an
exchange rate which already has a safety factor applied?
When converting from ETH to DOT, we need to _divide_ the multiplier by
the exchange rate, and to convert from DOT to ETH we need to _multiply_
the multiplier by the exchange rate.
> Other input parameters to the fee calculation can also deviate from
real-world values. These include substrate weights, gas prices, and so
on. Why does the multiplier introduced here not adjust those?
A single scalar multiplier won't be able to accommodate the different
volatilities efficiently. For example, gas prices are much more volatile
than exchange rates, and substrate weights hardly ever change.
So the pricing config relating to weights and gas prices should already
have some appropriate safety margin pre-applied.
# Detailed Changes:
* Added `multiplier` field to `PricingParameters`
* Outbound-queue fee is multiplied by `multiplier`
* This `multiplier` is synced to the Ethereum side
* Improved Runtime API for calculating outbound-queue fees. This API
makes it much easier to for configure parts of the system in preparation
for launch.
* Improve and clarify code documentation
Upstreamed from https://github.com/Snowfork/polkadot-sdk/pull/127
---------
Co-authored-by: Clara van Staden <claravanstaden64@gmail.com>
Co-authored-by: Adrian Catangiu <adrian@parity.io>
## Bug Explanation
Adds a check that prevents finalized headers with a gap larger than the
sync committee period being imported, which could cause execution
headers in the gap being unprovable. The current version of the Ethereum
client checks that there is a header at least every sync committee, but
it doesn't check that the headers are within a sync period of each
other. For example:
Header 100 (sync committee period 1)
Header 9000 (sync committee period 2)
(8900 blocks apart)
These headers are in adjacent sync committees, but more than the sync
committee period (8192 blocks) apart.
The reason we need a header every 8192 slots at least, is the header is
used to prove messages within the last 8192 blocks. If we import header
9000, and we receive a message to be verified at header 200, the
`block_roots` field of header 9000 won't contain the header in order to
do the ancestry check.
## Environment
While running in Rococo, this edge case was discovered after the relayer
was offline for a few days. It is unlikely, but not impossible, to
happen again and so it should be backported to polkadot-sdk 1.7.0 (so
that
[polkadot-fellows/runtimes](https://github.com/polkadot-fellows/runtimes)
can be updated with the fix).
Our Ethereum client has been operational on Rococo for the past few
months, and this been the only major issue discovered so far.
### Unrelated Change
An unrelated nit: Removes a left over file that should have been deleted
when the `parachain` directory was removed.
---------
Co-authored-by: claravanstaden <Cats 4 life!>
Based on issue
[#2512](https://github.com/paritytech/polkadot-sdk/issues/2512), it
seems that some ecosystem teams are using these networks to set up their
staging environments and test certain use cases, some of them involving
sending XCMs from the relay with origins not allowed in the current
configuration.
This change reverts the configuration of `SendXcmOrigin`.
---------
Co-authored-by: Adrian Catangiu <adrian@parity.io>
Small refactoring to reduce the algorithmic complexity of the initial
message distribution in approval voting after a sync from O(n_candidates
^ 2) to O(n_candidates).
We witnessed really poor performance on Rococo, where we ended up with
50 on-demand cores. This was due to the fact that for each core the full
queue was processed. With this change full queue processing will happen
way less often (most of the time complexity is O(1) or O(log(n))) and if
it happens then only for one core (in expectation).
Also spot price is now updated before each order to ensure economic back
pressure.
TODO:
- [x] Implement
- [x] Basic tests
- [x] Add more tests (see todos)
- [x] Run benchmark to confirm better performance, first results suggest
> 100x faster.
- [x] Write migrations
- [x] Bump scale-info version and remove patch in Cargo.toml
- [x] Write PR docs: on-demand performance improved, more on-demand
cores are now non problematic anymore. If need by also the max queue
size can be increased again. (Maybe not to 10k)
Optional: Performance can be improved even more, if we called
`pop_assignment_for_core()`, before calling `report_processed` (Avoid
needless affinity drops). The effect gets smaller the larger the claim
queue and I would only go for it, if it does not add complexity to the
scheduler.
---------
Co-authored-by: eskimor <eskimor@no-such-url.com>
Co-authored-by: antonva <anton.asgeirsson@parity.io>
Co-authored-by: command-bot <>
Co-authored-by: Anton Vilhelm Ásgeirsson <antonva@users.noreply.github.com>
Co-authored-by: ordian <write@reusable.software>
_This PR is being continued from
https://github.com/paritytech/polkadot-sdk/pull/2206, which was closed
when the developer_hub was merged._
closes https://github.com/paritytech/polkadot-sdk-docs/issues/44
---
# Description
This PR adds a reference document to the `developer-hub` crate (see
https://github.com/paritytech/polkadot-sdk/pull/2102). This specific
reference document covers defensive programming practices common within
the context of developing a runtime with Substrate.
In particular, this covers the following areas:
- Default behavior of how Rust deals with numbers in general
- How to deal with floating point numbers in runtime / fixed point
arithmetic
- How to deal with Integer overflows
- General "safe math" / defensive programming practices for common
pallet development scenarios
- Defensive traits that exist within Substrate, i.e.,
`defensive_saturating_add `, `defensive_unwrap_or`
- More general defensive programming examples (keep it concise)
- Link to relevant examples where these practices are actually in
production / being used
- Unwrapping (or rather lack thereof) 101
todo
--
- [x] Apply feedback from previous PR
- [x] This may warrant a PR to append some of these docs to
`sp_arithmetic`
---------
Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: Gonçalo Pestana <g6pestana@gmail.com>
Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
Co-authored-by: Francisco Aguirre <franciscoaguirreperez@gmail.com>
Co-authored-by: Radha <86818441+DrW3RK@users.noreply.github.com>
The PR adds two things:
1. Runtime API exposing the whole claim queue
2. Consumes the API in `collation-generation` to fetch the next
scheduled `ParaEntry` for an occupied core.
Related to https://github.com/paritytech/polkadot-sdk/issues/1797
Introduces `CryptoBytes` type defined as:
```rust
pub struct CryptoBytes<const N: usize, Tag = ()>(pub [u8; N], PhantomData<fn() -> Tag>);
```
The type implements a bunch of methods and traits which are typically
expected from a byte array newtype
(NOTE: some of the methods and trait implementations IMO are a bit
redundant, but I decided to maintain them all to not change too much
stuff in this PR)
It also introduces two (generic) typical consumers of `CryptoBytes`:
`PublicBytes` and `SignatureBytes`.
```rust
pub struct PublicTag;
pub PublicBytes<const N: usize, CryptoTag> = CryptoBytes<N, (PublicTag, CryptoTag)>;
pub struct SignatureTag;
pub SignatureBytes<const N: usize, CryptoTag> = CryptoBytes<N, (SignatureTag, CryptoTag)>;
```
Both of them use a tag to differentiate the two types at a higher level.
Downstream specializations will further specialize using a dedicated
crypto tag. For example in ECDSA:
```rust
pub struct EcdsaTag;
pub type Public = PublicBytes<PUBLIC_KEY_SERIALIZED_SIZE, EcdsaTag>;
pub type Signature = PublicBytes<PUBLIC_KEY_SERIALIZED_SIZE, EcdsaTag>;
```
Overall we have a cleaner and most importantly **consistent** code for
all the types involved
All these details are opaque to the end user which can use `Public` and
`Signature` for the cryptos as before
On top of #3302.
We want the validators to upgrade first before we add changes to the
collation side to send the new variants, which is why this part is
extracted into a separate PR.
The detection of when to send the parent head is based on the core
assignments at the relay parent of the candidate. We probably want to
make it more flexible in the future, but for now, it will work for a
simple use case when a para always has multiple cores assigned to it.
---------
Signed-off-by: Matteo Muraca <mmuraca247@gmail.com>
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Matteo Muraca <56828990+muraca@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Juan Ignacio Rios <54085674+JuaniRios@users.noreply.github.com>
Co-authored-by: Branislav Kontur <bkontur@gmail.com>
Co-authored-by: Bastian Köcher <git@kchr.de>
This PR enables the `transaction_unstable_broadcast ` and
`transaction_unstable_stop` RPC API.
Since the API is unstable, we don't need to expose this in the release
notes.
After merging this, we could validate the API in subxt and stabilize it.
Spec PR that stabilizes the API:
https://github.com/paritytech/json-rpc-interface-spec/pull/139
cc @paritytech/subxt-team
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Part of #3326
cc @kianenigma @ggwpez @liamaharon
polkadot address: 12poSUQPtcF1HUPQGY3zZu2P8emuW9YnsPduA4XG3oCEfJVp
---------
Signed-off-by: Matteo Muraca <mmuraca247@gmail.com>
Changes:
- Port the `pov_mode` attribute from the V1 syntax to V2
- Update `pallet-whitelist` and `frame-benchmarking-pallet-pov`
Follow up: also allow this attribute on top-level benchmark modules.
---------
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: command-bot <>
Currently the xcm-executor returns an `Unimplemented` error if it
receives any HRMP-related instruction.
What I propose here, which is what we are currently doing in our forked
executor at polimec, is to introduce a trait implemented by the executor
which will handle those instructions.
This way, if parachains want to keep the default behavior, they just use
`()` and it will return unimplemented, but they can also implement their
own logic to establish HRMP channels with other chains in an automated
fashion, without requiring to go through governance.
Our implementation is mentioned in the [polkadot HRMP
docs](https://arc.net/l/quote/hduiivbu), and it was suggested to us to
submit a PR to add these changes to polkadot-sdk.
---------
Co-authored-by: Branislav Kontur <bkontur@gmail.com>
Co-authored-by: command-bot <>
There is a problem in the way we update `authorithy-discovery` next keys
and because of that nodes that enter the active set would be noticed at
the start of the session they become active, instead of the start of the
previous session as it was intended. This is problematic because:
1. The node itself advertises its addresses on the DHT only when it
notices it should become active on around ~10m loop, so in this case it
would notice after it becomes active.
2. The other nodes won't be able to detect the new nodes addresses at
the beginning of the session, so it won't added them to the reserved
set.
With 1 + 2, we end-up in a situation where the the new node won't be
able to properly connect to its peers because it won't be in its peers
reserved set. Now, the nodes accept by default`MIN_GOSSIP_PEERS: usize =
25` connections to nodes that are not in the reserved set, but given
Kusama size(> 1000 nodes) you could easily have more than`25` new nodes
entering the active set or simply the nodes don't have slots anymore
because, they already have connections to peers not in the active set.
In the end what the node would notice is 0 backing rewards because it
wasn't directly connected to the peers in its backing group.
## Root-cause
The flow is like this:
1. At BAD_SESSION - 1, in `rotate_session` new nodes are added to
QueuedKeys
https://github.com/paritytech/polkadot-sdk/blob/02e1a7f476d7d7c67153e975ab9a1bdc02ffea12/substrate/frame/session/src/lib.rs#L609
```
<QueuedKeys<T>>::put(queued_amalgamated.clone());
<QueuedChanged<T>>::put(next_changed);
```
2. AuthorityDiscovery::on_new_session is called with `changed` being the
value of `<QueuedChanged<T>>:` at BAD_SESSION - **2** because it was
saved before being updated
https://github.com/paritytech/polkadot-sdk/blob/02e1a7f476d7d7c67153e975ab9a1bdc02ffea12/substrate/frame/session/src/lib.rs#L613
3. At BAD_SESSION - 1, `AuthorityDiscovery::on_new_session` doesn't
updated its next_keys because `changed` was false.
4. For the entire durations of `BAD_SESSION - 1` everyone calling
runtime api `authorities`(should return past, present and future
authorities) won't discover the nodes that should become active .
5. At the beginning of BAD_SESSION, all nodes discover the new nodes are
authorities, but it is already too late because reserved_nodes are
updated only at the beginning of the session by the `gossip-support`.
See above why this bad.
## Fix
Update next keys with the queued_validators at every session, not matter
the value of `changed` this is the same way babe pallet correctly does
it.
https://github.com/paritytech/polkadot-sdk/blob/02e1a7f476d7d7c67153e975ab9a1bdc02ffea12/substrate/frame/babe/src/lib.rs#L655
## Notes
- The issue doesn't reproduce with proof-authorities changes like
`versi` because `changed` would always be true and `AuthorityDiscovery`
correctly updates its next_keys every time.
- Confirmed at session `37651` on kusama that this is exactly what it
happens by looking at blocks with polkadot.js.
## TODO
- [ ] Move versi on proof of stake and properly test before and after
fix to confirm there is no other issue.
---------
Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
Co-authored-by: Bastian Köcher <git@kchr.de>
Removed the `pallet::getter` macro call from storage type definitions
and added the corresponding implementations directly.
fixes#3330
polkadot address: 14JzTPPUd8x8phKi8qLxHgNTnTMg6DUukCLXoWprejkaHXPz
---------
Co-authored-by: Bastian Köcher <git@kchr.de>
This PR removes sp-std crate from substrate/primitives sub-directories.
For now crates that have `pub use` of sp-std or export macros that would
necessitate users of the macros to `extern crate alloc` have been
excluded from this PR.
There should be no breaking changes in this PR.
---------
Co-authored-by: Koute <koute@users.noreply.github.com>
Crowdloan account should burn all funds after a crowd loan got dissolved
to ensure that the account is reaped correctly.
---------
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: Francisco Aguirre <franciscoaguirreperez@gmail.com>
Fixes#3128.
This introduces a new variant for the collation response from the
collator that includes the parent head data. For now, collators won't
send this new variant. We'll need to change the collator side of the
collator protocol to detect all the cores assigned to a para and send
the parent head data in the case when it's more than 1 core.
- [x] validate approach
- [x] check head data hash
Extracted Benchbuilder enhancements used in
https://github.com/paritytech/polkadot-sdk/pull/3644 . Might still
require some work to fully support all scenarios when disputing elastic
scaling parachains, but it should be useful in writing elastic scaling
runtime tests.
---------
Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>
Issues addressed in this PR:
- Improve *Penpal* runtime:
- Properly handled received assets. Previously, it treated `(1, Here)`
as the local native currency, whereas it should be treated as a
`ForeignAsset`. This wasn't a great example of standard Parachain
behaviour, as no Parachain treats the system asset as the local
currency.
- Remove `AllowExplicitUnpaidExecutionFrom` the system. Again, this
wasn't a great example of standard Parachain behaviour.
- Move duplicated
`ForeignAssetFeeAsExistentialDepositMultiplierFeeCharger` to
`assets_common` crate.
- Improve emulated tests:
- Update *Penpal* tests to new runtime.
- To simplify tests, register the reserve transferred, teleported, and
system assets in *Penpal* and *AssetHub* genesis. This saves us from
having to create the assets repeatedly for each test
- Add missing test case:
`reserve_transfer_assets_from_para_to_system_para`.
- Cleanup.
- Prevent integration tests crates imports from being re-exported, as
they were polluting the `polkadot-sdk` docs.
There is still a test case missing for reserve transfers:
- Reserve transfer of system asset from *Parachain* to *Parachain*
trough *AssetHub*.
- This is not yet possible with `pallet-xcm` due to the reasons
explained in https://github.com/paritytech/polkadot-sdk/pull/3339
---------
Co-authored-by: command-bot <>
Currently, the staking logic does not prevent a controller from becoming
a stash of *another* ledger (introduced by [removing this
check](https://github.com/paritytech/polkadot-sdk/pull/1484/files#diff-3aa6ceab5aa4e0ab2ed73a7245e0f5b42e0832d8ca5b1ed85d7b2a52fb196524L850)).
Given that the remaining of the code expects that never happens, bonding
a ledger with a stash that is a controller of another ledger may lead to
data inconsistencies and data losses in bonded ledgers. For more
detailed explanation of this issue:
https://hackmd.io/@gpestana/HJoBm2tqo/%2FTPdi28H7Qc2mNUqLSMn15w
In a nutshell, when fetching a ledger with a given controller, we may be
end up getting the wrong ledger which can lead to unexpected ledger
states.
This PR also ensures that `set_controller` does not lead to data
inconsistencies in the staking ledger and bonded storage in the case
when a controller of a stash is a stash of *another* ledger. and
improves the staking `try-runtime` checks to catch potential issues with
the storage preemptively.
In summary, there are two important cases here:
1. **"Sane" double bonded ledger**
When a controller of a ledger is a stash of *another* ledger. In this
case, we have:
```
> Bonded(stash, controller)
(A, B) // stash A with controller B
(B, C) // B is also a stash of another ledger
(C, D)
> Ledger(controller)
Ledger(B) = L_a (stash = A)
Ledger(C) = L_b (stash = B)
Ledger(D) = L_c (stash = C)
```
In this case, the ledgers can be mutated and all operations are OK.
However, we should not allow `set_controller` to be called if it means
it results in a "corrupt" double bonded ledger (see below).
3. **"Corrupt" double bonded ledger**
```
> Bonded(stash, controller)
(A, B) // stash A with controller B
(B, B)
(C, D)
```
In this case, B is a stash and controller AND is corrupted, since B is
responsible for 2 ledgers which is not correct and will lead to
inconsistent states. Thus, in this case, in this PR we are preventing
these ledgers from mutating (i.e. operations like bonding extra etc)
until the ledger is brought back to a consistent state.
---
**Changes**:
- Checks if stash is already a controller when calling `Call::bond`
(fixes the regression introduced by [removing this
check](https://github.com/paritytech/polkadot-sdk/pull/1484/files#diff-3aa6ceab5aa4e0ab2ed73a7245e0f5b42e0832d8ca5b1ed85d7b2a52fb196524L850));
- Ensures that all fetching ledgers from storage are done through the
`StakingLedger` API;
- Ensures that -- when fetching a ledger from storage using the
`StakingLedger` API --, a `Error::BadState` is returned if the ledger
bonding is in a bad state. This prevents bad ledgers from mutating (e.g.
`bond_extra`, `set_controller`, etc) its state and avoid further data
inconsistencies.
- Prevents stashes which are controllers or another ledger from calling
`set_controller`, since that may lead to a bad state.
- Adds further try-state runtime checks that check if there are ledgers
in a bad state based on their bonded metadata.
Related to https://github.com/paritytech/polkadot-sdk/issues/3245
---------
Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
Co-authored-by: kianenigma <kian@parity.io>
Make Rococo & Westend XCM's location converter `HashedDescription` more
in line with Polkadot/Kusama runtimes.
Co-authored-by: Muharem <ismailov.m.h@gmail.com>
eskimor