Conversation
| let network = EthereumNetwork::get(); | ||
|
|
||
| let fee_asset = Location::new(1, Here); | ||
| let fee_value = 1_000_000_000u128; // TODO get from command |
There was a problem hiding this comment.
There was a problem hiding this comment.
@vgeddes added the following:
- Xcm prologue fee to config: https://github.com/claravanstaden/polkadot-sdk/pull/4/files#diff-bfe761b371a553619edf5be2dd792c45bd14f1a916e51b77f5b77b9395d92cdaR110
- Burn fees from relayer account: https://github.com/claravanstaden/polkadot-sdk/pull/4/files#diff-bfe761b371a553619edf5be2dd792c45bd14f1a916e51b77f5b77b9395d92cdaR218
- Test to calculate AH execution fee, added 2x buffer: https://github.com/claravanstaden/polkadot-sdk/pull/4/files#diff-8397e49665ab41c48eb0755a8a6b2510bbe94b0c00d808e6d167c3b0e2d0db26R26
| RefundSurplus, | ||
| // Refund excess fees to the relayer | ||
| // TODO maybe refund all fees to the relayer instead of just DOT? | ||
| DepositAsset { | ||
| assets: Wild(AllOf { id: AssetId(fee_asset.into()), fun: WildFungible }), | ||
| beneficiary: origin_account_location, | ||
| }, |
There was a problem hiding this comment.
If the extra fee(in WETH) is refunded to the relayer, we may not need to deposit it into RewardLeger?
There was a problem hiding this comment.
Hmmm are they not separate though? Ie the reward is specified here: https://github.com/claravanstaden/polkadot-sdk/pull/4/files#diff-51b54cea1a87bc694f384034a09cd94281e472795011f662bdb106deb9db2699R23 For example, say the fee is 1 WETH and the execution fee is 0.2 WETH, fee provided was 0.3 WETH. So 0.1 WETH will be refunded but the relayer should actually received the full 1 WETH?
There was a problem hiding this comment.
say the fee is 1 WETH, ... fee provided was 0.3 WETH
I'd assume fee provided should be same as the Ether value from envelop which is already charged from user.
It should cover all the cost/reward including, then we don't need a separated reward process?
cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/bridge_to_ethereum_config.rs
Outdated
Show resolved
Hide resolved
|
@alistair-singh when you have a chance please also review. |
cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/xcm_config.rs
Outdated
Show resolved
Hide resolved
| let fee: xcm::prelude::Asset = (fee_asset.clone(), XcmPrologueFee::get()).into(); | ||
| let mut instructions = vec![ | ||
| ReceiveTeleportedAsset(fee.clone().into()), | ||
| PayFees { asset: fee }, |
There was a problem hiding this comment.
Not sure this will work for advanced use cases like 2 hops transfer or transact.
e.g. In V1 for register ERC20 on AH there is a create_asset fee required we need to prefund the bridge location which acts as the owner of the asset.
Or what will happen if the transact is very expensive which cost more than the XcmPrologueFee? Is that possible to refill the fee registry with user provided xcm?
There was a problem hiding this comment.
As I said before, the fees for transact/create_asset will be covered by assets in message.assets and ExchangeAsset/PayFee instructions in message.xcm
There was a problem hiding this comment.
Specifically, follow this in code in Contracts V2 for issuing token registration messages: https://github.com/Snowfork/snowbridge/blob/553b0b9d9ec6f3683c6c605417ea5532282771ed/contracts/src/v2/Calls.sol#L115
There was a problem hiding this comment.
Could we add a simulated test to demonstrate register WETH on AH?
There was a problem hiding this comment.
@claravanstaden @vgeddes
Since user already paid Ether on Ethereum, why not just use it as fee? Then we don't need burn/teleport on BH from the relayer.
It's also consistent with Polkadot->Ethereum direction which we always stick to WETH as fee.
#5 for demonstration, please check if that make sense.
There was a problem hiding this comment.
I think its safer to use DOT for the the prologue fee. Under the hood, if the executor cannot exchange the WETH for DOT to pay for fees, then the XCM prologue will fail and the message.assets would be lost and unclaimable.
The relayer needs to provide DOT on BH for transaction fees anyway. Teleporting a few cents of DOT to AH for prologue fees isn't a big deal, I'd say.
It also makes accounting simpler. All of the Ether in the message is used for the relayer reward, instead of trying to split it up.
There was a problem hiding this comment.
if the executor cannot exchange the WETH for DOT to pay for fees
AH will just accept any sufficient foreign asset as fee, as we can fee from the smoke test in my PR there is even no ExchangeAsset instruction required.
XCM prologue will fail and the message.assets would be lost and unclaimable.
This won't happen since we've already added SetClaimer instruction?
All of the Ether in the message is used for the relayer reward, instead of trying to split it up.
No need to split it up, just refund all extra fees to the relayer which is the reward. Then we don't even need a separated round of the reward process. More details in #4 (comment)
There was a problem hiding this comment.
AH will just accept any sufficient foreign asset as fee
These are always exchanged to Dot behind the scenes automatically. If the weth/dot liquidity pool is empty/broken there will be a failure.
Then we don't even need a separated round of the reward process
I prefer having a separate reward process. It has several benefits, like giving the relayer the option of choosing their account where claimed rewards can be deposited.
There was a problem hiding this comment.
If the weth/dot liquidity pool is empty/broken there will be a failure.
Btw: weth/dot liquidity pool is not required on AH, there are multiple traders configured there.
https://github.com/paritytech/polkadot-sdk/blob/2ef2723126584dfcd6d2a9272282ee78375dbcd3/cumulus/parachains/runtimes/assets/asset-hub-westend/src/xcm_config.rs#L437-L449 use ED as exchange rate by default.
cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/bridge_to_ethereum_config.rs
Outdated
Show resolved
Hide resolved
cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/bridge_to_ethereum_config.rs
Outdated
Show resolved
Hide resolved
| let fee: xcm::prelude::Asset = (fee_asset.clone(), XcmPrologueFee::get()).into(); | ||
| let mut instructions = vec![ | ||
| ReceiveTeleportedAsset(fee.clone().into()), | ||
| PayFees { asset: fee }, |
There was a problem hiding this comment.
I think its safer to use DOT for the the prologue fee. Under the hood, if the executor cannot exchange the WETH for DOT to pay for fees, then the XCM prologue will fail and the message.assets would be lost and unclaimable.
The relayer needs to provide DOT on BH for transaction fees anyway. Teleporting a few cents of DOT to AH for prologue fees isn't a big deal, I'd say.
It also makes accounting simpler. All of the Ether in the message is used for the relayer reward, instead of trying to split it up.
| let register_token_xcm = | ||
| vec![ | ||
| ExchangeAsset { give: weth_fee.into(), want: dot_fee.clone().into(), maximal: false }, | ||
| PayFees { asset: dot_fee }, |
There was a problem hiding this comment.
Should check with Cisco/Adrian, but on AH it should be possible to pay fees directly with WETH without having to exchange first.
I guess either way, you can test this using an xcm simulation test.
There was a problem hiding this comment.
Now that you mention it, I tested the automatic exchange from Weth to DOT with the unified rewards work - it works.
Register token xcm: 0508310100000700b2118417060315013500020209049edaa80203005861980c0268778f56f803753e3892a2fb341a5300e5eaf816d3384d366e1eed03a1f0bf67823068c95947dae4455b020216f0cf6101000000
Replace token ID: 0x5861980c0268778f56f803753e3892a2fb341a53
Replace bridge owner ID: 0xe5eaf816d3384d366e1eed03a1f0bf67823068c95947dae4455b020216f0cf61
There was a problem hiding this comment.
Nice!
owner should be constructed the same as in v1 though:
let owner = GlobalConsensusEthereumConvertsFor::<[u8; 32]>::from_chain_id(&chain_id);
There was a problem hiding this comment.
That's a good point!
token: 0xc02aaa39b223fe8d0a0e5c4f27ead9083c756cc2
Register token hex Sepolia: 0508310100000700b2118417060315013500020209049edaa8020300c02aaa39b223fe8d0a0e5c4f27ead9083c756cc200ce796ae65569a670d0c1cc1ac12515a3ce21b5fbf729d63d7b289baad070139d01000000
Register token Mainnet: 0508310100000700b2118417060315013500020209049edaa8020300c02aaa39b223fe8d0a0e5c4f27ead9083c756cc200204dfe37731e8e2b4866ad0da9a17c49f434542c3477c5f914a3349acd88ba1a01000000
There was a problem hiding this comment.
Looks like you're still passing the dot fee though: PayFees { asset: dot_fee },. Shouldn't it be weth?
There was a problem hiding this comment.
Hmmm I don't know. We provide a weth fee, but the asset creation fee amount is in dot. So perhaps we should specify a dot fee here and the weth will be traded for dot? Otherwise we again need to keep some exchange rate to specify a sufficient weth amount here in PayFees. Correct me if I am wrong.
There was a problem hiding this comment.
Added ExchangeAsset back: https://github.com/claravanstaden/polkadot-sdk/pull/4/files#diff-8397e49665ab41c48eb0755a8a6b2510bbe94b0c00d808e6d167c3b0e2d0db26R160
Updated xcm for Sepolia:
050c0f0004020209049edaa8020300774667629726ec1FaBEbCEc0D9139bD1C8f72a23000f00c06e31d91001040100000700b211841700310100000700b2118417060315013500020209049edaa8020300c02aaa39b223fe8d0a0e5c4f27ead9083c756cc200ce796ae65569a670d0c1cc1ac12515a3ce21b5fbf729d63d7b289baad070139d01000000
We need to replace these three values in the contract xcm:
- Weth Token ID:
0x774667629726ec1FaBEbCEc0D9139bD1C8f72a23 - Weth amount (to be provided to exchange for DOT to cover the asset creation fee):
0x00c06e31d91001 - Token ID (registered asset contract address):
0xc02aaa39b223fe8d0a0e5c4f27ead9083c756cc2
There was a problem hiding this comment.
Thanks Clara. I suspect we're going to be iterating on the registration XCM for a while, and I so I think the unit test:
- Needs to fleshed out into a fully simulated XCM test
- The test should automatically generate the byte fragments. See example code I asked Claude AI to produce.
bridges/snowbridge/pallets/inbound-queue-v2/runtime-api/src/lib.rs
Outdated
Show resolved
Hide resolved
bridges/snowbridge/pallets/inbound-queue-v2/runtime-api/src/lib.rs
Outdated
Show resolved
Hide resolved
bridges/snowbridge/pallets/inbound-queue-v2/runtime-api/src/lib.rs
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| #[test] | ||
| fn register_token_xcm() { |
There was a problem hiding this comment.
I do think we need to upgrade this to a fully simulated test that verifies that the xcm is executed properly on AH.
Can do this using the EthereumInboundQueueV2::send_xcm helper surely?
| println!("register token id: {:x?}", token); | ||
| println!("weth token id: {:x?}", weth_token_id); | ||
| println!("weth_amount: {:x?}", hex::encode(weth_amount.encode())); | ||
| println!("dot asset: {:x?}", hex::encode(dot_fee.encode())); |
There was a problem hiding this comment.
Do you want to specify the DOT amount in the contract as well? That should stay constant given the runtime config does not change.
…#6636) # Description These changes should enhance the quality of benchmark results by excluding worker initialization time from the measurements and reducing the overall duration of the benchmarks. ### Integration It should not affect any downstream projects. ### Review Notes - Workers initialize once per benchmark to avoid side effects. - The listen address is assigned when a worker starts. - Benchmarks are divided into two groups by size to create better charts for comparison. --------- Co-authored-by: GitHub Action <action@github.com>
…tytech#6760) This PR ensures that substrate always reports discarded items as zero. This is needed to align with the rpc-v2 spec Closes: paritytech#6683 cc @paritytech/subxt-team --------- Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io> Co-authored-by: GitHub Action <action@github.com>
Re-enable zombienet test for `solochain`. Thx!
We were trapping the host context in case a sub call was exhausting the storage deposit limit set for this sub call. This prevents the caller from handling this error. In this PR we added a new error code that is returned when either gas or storage deposit limit is exhausted by the sub call. We also remove the longer used `NotCallable` error. No longer used because this is no longer an error: It will just be a balance transfer. We also make `set_code_hash` infallible to be consistent with other host functions which just trap on any error condition. --------- Co-authored-by: GitHub Action <action@github.com>
Fix adds release environment to the backport job, so that token could be properly generated
…ns (paritytech#6643) Closes: paritytech#6585 Removing the `require_weight_at_most` parameter in V5 Transact had only one problem. Converting a message from V5 to V4 to send to chains that didn't upgrade yet. The conversion would not know what weight to give to the Transact, since V4 and below require it. To fix this, I added back the weight in the form of an `Option<Weight>` called `fallback_max_weight`. This can be set to `None` if you don't intend to deal with a chain that hasn't upgraded yet. If you set it to `Some(_)`, the behaviour is the same. The plan is to totally remove this in V6 since there will be a good conversion path from V6 to V5. --------- Co-authored-by: GitHub Action <action@github.com> Co-authored-by: Adrian Catangiu <adrian@parity.io>
…ound-queue-v2-clean
Fixed condition which sets weights/large images
Currently the `para_backing_state` API is used only by the prospective parachains subsystems and returns 2 things: the constraints for parachain blocks and the candidates pending availability. This PR deprecates `para_backing_state` and introduces a new `backing_constraints` API that can be used together with `candidates_pending_availability` to get the same information provided by `para_backing_state`. TODO: - [x] PRDoc --------- Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> Co-authored-by: command-bot <>
Closes paritytech#6196 Closes paritytech#7204 Example of PR: paritytech#6816 Every sunday 01:00 AM it's going to start to benchmark (with /cmd bench) all runtimes and all pallets Then diff total will be pushed to a branch and PR open,. I assume review-bot is going assign required reviewers per changed files I afraid each weeks will be too much to review & merge, but we can adjust later Bonus: fix for pallet_multisig lib and substrate/.maintain/frame-weight-template.hbs , which didn't let to compile new weights --------- Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> Co-authored-by: command-bot <> Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Closes: paritytech#2904 --------- Co-authored-by: command-bot <>
…tytech#7299) # Description This PR builds frame-omni-bencher with `production` profile when calling `/cmd bench-omni` to compute benchmarks for pallets. Fix proposed by @bkchr , thanks! Closes paritytech#6797. ## Integration N/A ## Review Notes More info on paritytech#6797, and related to how the fix was tested: paritytech#6797 (comment). --------- Signed-off-by: Iulian Barbu <iulian.barbu@parity.io> Co-authored-by: command-bot <>
This PR includes minor fixes identified during work on the larger PR: [https://github.com/paritytech/polkadot-sdk/issues/6906](https://github.com/paritytech/polkadot-sdk/issues/6906). Specifically, this PR removes the use of `open_bridge_between_asset_hub_rococo_and_asset_hub_westend`, which is no longer relevant for BridgeHubs, as bridges are now created with genesis settings. This function was used in the generic `test_dry_run_transfer_across_pk_bridge` macro, which could cause compilation issues when used in other contexts (e.g. fellows repo). --------- Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…h#7227) # Description We're unable to sync templates repos with what's in polkadot-sdk/templates for stable2412 because the tag which references the release (`polkadot-stable2412`) is missing the Plan.toml file, which is needed by PSVM, ran when syncing, to update the templates dependencies versions in Cargo.tomls. This PR adds a workflow `patch` input, to enable the workflow to use PSVM with a tag corresponding to a patch stable release (e.g. `polkadot-stable2412-1`), which will contain the `Plan.toml` file. ## Integration This enables the templates repos update with the contents of latest stable2412 release, in terms of polkadot-sdk/templates, which is relevant for getting-started docs. ## Review Notes This PR adds a `patch` input for the `misc-sync-templates.yml` workflow, which if set will be used with `psvm` accordingly to update templates repos' dependencies versions based on upcomming patch stable2412-1, which contains the `Plan.toml`. The workflow will be ran manually after stable2412-1 is out and this work is tracked under paritytech#6329 . Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
- Add option to specify database_url using DATABASE_URL environment variable - Add a eth-rpc-tester rust bin that can be used to test deployment before releasing eth-rpc - make evm_block non fallible so that it can return an Ok response for older blocks when the runtime API is not available - update cargo.lock to integrate changes from paritytech/subxt#1904 --------- Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>
… set (paritytech#7318) This regression was introduced with some of the recent PRs. Regression fixed and test added. --------- Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>
The link to Polkachu is not working
…n the pre dispatch is zero. (paritytech#7327) Related paritytech#6772 For an extrinsic, in the post dispatch info, the actual weight is only used to reclaim unused weight. If the actual weight is more than the pre dispatch weight, then the extrinsic is using the minimum, e.g., the weight used registered in pre dispatch. In parachain-system pallet one call is `set_validation_data`. This call is returning an actual weight, but the pre-dispatch weight is 0. This PR fix the disregard of actual weight of `set_validation_data` by registering it manually.
…vent` and `assert_has_event` (paritytech#7142) Without track caller the error message of the assert points to the `assert_last_event` function, which is not useful. ``` thread 'tests::set_metadata_works' panicked at /home/gui/Developpement/polkadot-sdk/substrate/frame/system/src/lib.rs:2034:9: assertion `left == right` failed: expected event RuntimeEvent::Referenda(Event::MetadataSet { index: 0, hash: 0xbb30a42c1e62f0afda5f0a4e8a562f7a13a24cea00ee81917b86b89e801314aa }) is not equal to the last event RuntimeEvent::Referenda(Event::MetadataSet { index: 1, hash: 0xbb30a42c1e62f0afda5f0a4e8a562f7a13a24cea00ee81917b86b89e801314aa }) left: RuntimeEvent::Referenda(Event::MetadataSet { index: 1, hash: 0xbb30a42c1e62f0afda5f0a4e8a562f7a13a24cea00ee81917b86b89e801314aa }) right: RuntimeEvent::Referenda(Event::MetadataSet { index: 0, hash: 0xbb30a42c1e62f0afda5f0a4e8a562f7a13a24cea00ee81917b86b89e801314aa }) ``` With the track caller the error message points to the caller, showing the source of the error: ``` thread 'tests::set_metadata_works' panicked at substrate/frame/referenda/src/tests.rs:639:9: assertion `left == right` failed: expected event RuntimeEvent::Referenda(Event::MetadataSet { index: 0, hash: 0xbb30a42c1e62f0afda5f0a4e8a562f7a13a24cea00ee81917b86b89e801314aa }) is not equal to the last event RuntimeEvent::Referenda(Event::MetadataSet { index: 1, hash: 0xbb30a42c1e62f0afda5f0a4e8a562f7a13a24cea00ee81917b86b89e801314aa }) left: RuntimeEvent::Referenda(Event::MetadataSet { index: 1, hash: 0xbb30a42c1e62f0afda5f0a4e8a562f7a13a24cea00ee81917b86b89e801314aa }) right: RuntimeEvent::Referenda(Event::MetadataSet { index: 0, hash: 0xbb30a42c1e62f0afda5f0a4e8a562f7a13a24cea00ee81917b86b89e801314aa }) ``` I also improved the error message to include a warning when checking events on block number zero.
…rd-compatible) (paritytech#7344) Revert paritytech#7011 and replace it with a backward-compatible solution suitable for backporting to a release branch. ### Review notes It's easier to review this PR per commit: the first commit is just a revert, so it's enough to review only the second one, which is almost a one-liner. --------- Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>
closes paritytech#5978 --------- Co-authored-by: command-bot <> Co-authored-by: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com>
…mni Node compatibility (paritytech#6529) # Description This PR adds development chain specs for the minimal and parachain templates. [paritytech#6334](paritytech#6334) ## Integration This PR adds development chain specs for the minimal and para chain template runtimes, ensuring synchronization with runtime code. It updates zombienet-omni-node.toml, zombinet.toml files to include valid chain spec paths, simplifying configuration for zombienet in the parachain and minimal template. ## Review Notes 1. Overview of Changes: - Added development chain specs for use in the minimal and parachain template. - Updated zombienet-omni-node.toml and zombinet.toml files in the minimal and parachain templates to include paths to the new dev chain specs. 2. Integration Guidance: **NB: Follow the templates' READMEs from the polkadot-SDK master branch. Please build the binaries and runtimes based on the polkadot-SDK master branch.** - Ensure you have set up your runtimes `parachain-template-runtime` and `minimal-template-runtime` - Ensure you have installed the nodes required ie `parachain-template-node` and `minimal-template-node` - Set up [Zombinet](https://paritytech.github.io/zombienet/intro.html) - For running the parachains, you will need to install the polkadot `cargo install --path polkadot` remember from the polkadot-SDK master branch. - Inside the template folders minimal or parachain, run the command to start with `Zombienet with Omni Node`, `Zombienet with minimal-template-node` or `Zombienet with parachain-template-node` *Include your leftover TODOs, if any, here.* * [ ] Test the syncing of chain specs with runtime's code. --------- Signed-off-by: EleisonC <ckalule7@gmail.com> Co-authored-by: Iulian Barbu <14218860+iulianbarbu@users.noreply.github.com> Co-authored-by: Alexander Samusev <41779041+alvicsam@users.noreply.github.com>
…h#6614) #Description Migrated polkadot-runtime-parachains slots benchmarking to the new benchmarking syntax v2. This is part of paritytech#6202 --------- Co-authored-by: Giuseppe Re <giuseppe.re@parity.io> Co-authored-by: seemantaggarwal <32275622+seemantaggarwal@users.noreply.github.com> Co-authored-by: Bastian Köcher <git@kchr.de>
Co-authored-by: Francisco Aguirre <franciscoaguirreperez@gmail.com>
Co-authored-by: Francisco Aguirre <franciscoaguirreperez@gmail.com>
…b.rs Co-authored-by: Adrian Catangiu <adrian@parity.io>
Co-authored-by: Adrian Catangiu <adrian@parity.io>
Co-authored-by: Adrian Catangiu <adrian@parity.io>
Co-authored-by: Adrian Catangiu <adrian@parity.io>
Uh oh!
There was an error while loading. Please reload this page.