[Encointer] fix treasury asset payment#944
Conversation
| ReportError(QueryResponseInfo { | ||
| destination: destination.clone(), | ||
| query_id, | ||
| max_weight: Weight::zero(), | ||
| }), |
There was a problem hiding this comment.
First, I thought that this is just a max_weight issue, so I changed this to max. However, I still got a NotHoldingFees error, although I surely withdraw enough as shown in the logs.
I have fixed the problem for now, but I guess that this might even be an XCM bug such that ReportError doesn't the appendix together with DescendOrigin yet? What do the XCM overlords think @franciscoaguirre, @bkontur, @acatangiu?
XCM Execution Logs
2025-10-04T07:13:08.321016Z TRACE xcm::process: origin=Some(Location { parents: 1, interior: X1([Parachain(1001)]) }) total_surplus=Weight { ref_time: 0, proof_size: 0 } total_refunded=Weight { ref_time: 0, proof_size: 0 } error_handler_weight=Weight { ref_time: 0, proof_size: 0 }
2025-10-04T07:13:08.321058Z TRACE xcm::process_instruction: Processing instruction instruction=DescendOrigin(X1([AccountId32 { network: None, id: [150, 141, 187, 98, 102, 33, 87, 174, 108, 105, 38, 201, 33, 252, 99, 215, 105, 11, 253, 230, 89, 13, 87, 138, 18, 41, 154, 220, 108, 179, 239, 229] }]))
2025-10-04T07:13:08.321082Z TRACE xcm::process_instruction: Processing instruction instruction=WithdrawAsset(Assets([Asset { id: AssetId(Location { parents: 1, interior: Here }), fun: Fungible(12905733320) }]))
2025-10-04T07:13:08.321096Z TRACE xcm::ensure_can_subsume_assets: Ensuring subsume assets work worst_case_holding_len=1 holding_limit=64
2025-10-04T07:13:08.321269Z TRACE xcm::fungible_adapter: withdraw_asset what=Asset { id: AssetId(Location { parents: 1, interior: Here }), fun: Fungible(12905733320) } who=Location { parents: 1, interior: X2([Parachain(1001), AccountId32 { network: None, id: [150, 141, 187, 98, 102, 33, 87, 174, 108, 105, 38, 201, 33, 252, 99, 215, 105, 11, 253, 230, 89, 13, 87, 138, 18, 41, 154, 220, 108, 179, 239, 229] }]) }
2025-10-04T07:13:08.323721Z TRACE xcm::process_instruction: Processing instruction instruction=PayFees { asset: Asset { id: AssetId(Location { parents: 1, interior: Here }), fun: Fungible(12905733320) } }
2025-10-04T07:13:08.323749Z TRACE xcm::executor::PayFees: asset_for_fees=Asset { id: AssetId(Location { parents: 1, interior: Here }), fun: Fungible(12905733320) } message_weight=Weight { ref_time: 43793552000, proof_size: 387172 }
2025-10-04T07:13:08.323939Z TRACE xcm::weight: UsingComponents::buy_weight weight=Weight { ref_time: 43793552000, proof_size: 387172 } payment=AssetsInHolding { fungible: {AssetId(Location { parents: 1, interior: Here }): 12905733320}, non_fungible: {} } context=XcmContext { origin: Some(Location { parents: 1, interior: X2([Parachain(1001), AccountId32 { network: None, id: [150, 141, 187, 98, 102, 33, 87, 174, 108, 105, 38, 201, 33, 252, 99, 215, 105, 11, 253, 230, 89, 13, 87, 138, 18, 41, 154, 220, 108, 179, 239, 229] }]) }), message_id: [207, 224, 150, 20, 104, 44, 231, 40, 113, 150, 204, 227, 199, 74, 162, 200, 138, 143, 3, 176, 227, 143, 46, 145, 94, 71, 215, 205, 7, 177, 227, 162], topic: None }
2025-10-04T07:13:08.323991Z TRACE xcm::buy_weight: Buy weight succeeded weight_trader=staging_xcm_builder::weight::UsingComponents<system_parachains_constants::kusama::fee::WeightToFee, asset_hub_kusama_runtime::xcm_config::KsmLocation, sp_core::crypto::AccountId32, pallet_balances::pallet::Pallet<asset_hub_kusama_runtime::Runtime>, frame_support::traits::tokens::imbalance::on_unbalanced::ResolveTo<asset_hub_kusama_runtime::xcm_config::StakingPot, pallet_balances::pallet::Pallet<asset_hub_kusama_runtime::Runtime>>>
2025-10-04T07:13:08.324157Z TRACE xcm::process_instruction: Processing instruction instruction=SetAppendix(Xcm([ReportError(QueryResponseInfo { destination: Location { parents: 1, interior: X1([Parachain(1001)]) }, query_id: 0, max_weight: Weight { ref_time: 18446744073709551615, proof_size: 18446744073709551615 } }), RefundSurplus, DepositAsset { assets: Wild(All), beneficiary: Location { parents: 1, interior: X2([Parachain(1001), AccountId32 { network: None, id: [150, 141, 187, 98, 102, 33, 87, 174, 108, 105, 38, 201, 33, 252, 99, 215, 105, 11, 253, 230, 89, 13, 87, 138, 18, 41, 154, 220, 108, 179, 239, 229] }]) } }]))
2025-10-04T07:13:08.324182Z TRACE xcm::weight: WeightInfoBounds Xcm([ReportError(QueryResponseInfo { destination: Location { parents: 1, interior: X1([Parachain(1001)]) }, query_id: 0, max_weight: Weight { ref_time: 18446744073709551615, proof_size: 18446744073709551615 } }), RefundSurplus, DepositAsset { assets: Wild(All), beneficiary: Location { parents: 1, interior: X2([Parachain(1001), AccountId32 { network: None, id: [150, 141, 187, 98, 102, 33, 87, 174, 108, 105, 38, 201, 33, 252, 99, 215, 105, 11, 253, 230, 89, 13, 87, 138, 18, 41, 154, 220, 108, 179, 239, 229] }]) } }])
2025-10-04T07:13:08.324225Z TRACE xcm::process_instruction: Processing instruction instruction=TransferAsset { assets: Assets([Asset { id: AssetId(Location { parents: 0, interior: X2([PalletInstance(50), GeneralIndex(1984)]) }), fun: Fungible(10000000) }]), beneficiary: Location { parents: 0, interior: X1([AccountId32 { network: None, id: [5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5] }]) } }
2025-10-04T07:13:08.324402Z TRACE xcm::fungible_adapter: internal_transfer_asset what=Asset { id: AssetId(Location { parents: 0, interior: X2([PalletInstance(50), GeneralIndex(1984)]) }), fun: Fungible(10000000) } from=Location { parents: 1, interior: X2([Parachain(1001), AccountId32 { network: None, id: [150, 141, 187, 98, 102, 33, 87, 174, 108, 105, 38, 201, 33, 252, 99, 215, 105, 11, 253, 230, 89, 13, 87, 138, 18, 41, 154, 220, 108, 179, 239, 229] }]) } to=Location { parents: 0, interior: X1([AccountId32 { network: None, id: [5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5] }]) }
2025-10-04T07:13:08.324446Z TRACE xcm::fungibles_adapter: internal_transfer_asset what=Asset { id: AssetId(Location { parents: 0, interior: X2([PalletInstance(50), GeneralIndex(1984)]) }), fun: Fungible(10000000) } from=Location { parents: 1, interior: X2([Parachain(1001), AccountId32 { network: None, id: [150, 141, 187, 98, 102, 33, 87, 174, 108, 105, 38, 201, 33, 252, 99, 215, 105, 11, 253, 230, 89, 13, 87, 138, 18, 41, 154, 220, 108, 179, 239, 229] }]) } to=Location { parents: 0, interior: X1([AccountId32 { network: None, id: [5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5] }]) }
2025-10-04T07:13:08.328370Z TRACE xcm::process_instruction: Processing instruction instruction=SetTopic([207, 224, 150, 20, 104, 44, 231, 40, 113, 150, 204, 227, 199, 74, 162, 200, 138, 143, 3, 176, 227, 143, 46, 145, 94, 71, 215, 205, 7, 177, 227, 162])
2025-10-04T07:13:08.328393Z TRACE xcm::execute: Message executed result=Ok(())
2025-10-04T07:13:08.328407Z TRACE xcm::process: origin=Some(Location { parents: 1, interior: X2([Parachain(1001), AccountId32 { network: None, id: [150, 141, 187, 98, 102, 33, 87, 174, 108, 105, 38, 201, 33, 252, 99, 215, 105, 11, 253, 230, 89, 13, 87, 138, 18, 41, 154, 220, 108, 179, 239, 229] }]) }) total_surplus=Weight { ref_time: 0, proof_size: 0 } total_refunded=Weight { ref_time: 0, proof_size: 0 } error_handler_weight=Weight { ref_time: 0, proof_size: 0 }
2025-10-04T07:13:08.328434Z TRACE xcm::process_instruction: Processing instruction instruction=ReportError(QueryResponseInfo { destination: Location { parents: 1, interior: X1([Parachain(1001)]) }, query_id: 0, max_weight: Weight { ref_time: 18446744073709551615, proof_size: 18446744073709551615 } })
2025-10-04T07:13:08.328552Z TRACE xcm::send: Sending msg msg=Xcm([QueryResponse { query_id: 0, response: ExecutionResult(None), max_weight: Weight { ref_time: 18446744073709551615, proof_size: 18446744073709551615 }, querier: Some(Location { parents: 0, interior: X1([AccountId32 { network: None, id: [150, 141, 187, 98, 102, 33, 87, 174, 108, 105, 38, 201, 33, 252, 99, 215, 105, 11, 253, 230, 89, 13, 87, 138, 18, 41, 154, 220, 108, 179, 239, 229] }]) }) }, SetTopic([207, 224, 150, 20, 104, 44, 231, 40, 113, 150, 204, 227, 199, 74, 162, 200, 138, 143, 3, 176, 227, 143, 46, 145, 94, 71, 215, 205, 7, 177, 227, 162])]) destination=Location { parents: 1, interior: X1([Parachain(1001)]) } reason=Report
2025-10-04T07:13:08.328888Z TRACE xcm::pallet_xcm::note_unknown_version: XCM version is unknown for destination dest=Location { parents: 1, interior: X1([Parachain(1001)]) }
2025-10-04T07:13:08.329239Z TRACE xcm::contains: AllSiblingSystemParachains location: Location { parents: 1, interior: X2([Parachain(1001), AccountId32 { network: None, id: [150, 141, 187, 98, 102, 33, 87, 174, 108, 105, 38, 201, 33, 252, 99, 215, 105, 11, 253, 230, 89, 13, 87, 138, 18, 41, 154, 220, 108, 179, 239, 229] }]) }
2025-10-04T07:13:08.329272Z TRACE xcm::fees: Taking fees fees=Assets([Asset { id: AssetId(Location { parents: 1, interior: Here }), fun: Fungible(1030999968) }]) origin_ref=Some(Location { parents: 1, interior: X2([Parachain(1001), AccountId32 { network: None, id: [150, 141, 187, 98, 102, 33, 87, 174, 108, 105, 38, 201, 33, 252, 99, 215, 105, 11, 253, 230, 89, 13, 87, 138, 18, 41, 154, 220, 108, 179, 239, 229] }]) }) fees_mode=FeesMode { jit_withdraw: false } reason=Report
2025-10-04T07:13:08.329308Z TRACE xcm::fees: asset_to_pay_for_fees=Asset { id: AssetId(Location { parents: 1, interior: Here }), fun: Fungible(1030999968) }
2025-10-04T07:13:08.329331Z ERROR xcm::fees: Holding doesn't hold enough for fees e=AssetUnderflow(Asset { id: AssetId(Location { parents: 1, interior: Here }), fun: Fungible(1030999968) }) asset_to_pay_for_fees=Asset { id: AssetId(Location { parents: 1, interior: Here }), fun: Fungible(1030999968) }
2025-10-04T07:13:08.329352Z DEBUG xcm::process: XCM execution failed at instruction index=0 error=NotHoldingFees
2025-10-04T07:13:08.329711Z TRACE xcm::execute: Message executed result=Err(ExecutorError { index: 0, xcm_error: NotHoldingFees, weight: Weight { ref_time: 41967000000, proof_size: 367500 } })
2025-10-04T07:13:08.329740Z TRACE xcm::refund_surplus: Refunding surplus total_surplus=Weight { ref_time: 41967000000, proof_size: 367500 } total_refunded=Weight { ref_time: 0, proof_size: 0 } current_surplus=Weight { ref_time: 41967000000, proof_size: 367500 }
2025-10-04T07:13:08.329756Z TRACE xcm::weight: UsingComponents::refund_weight weight=Weight { ref_time: 41967000000, proof_size: 367500 } context=XcmContext { origin: Some(Location { parents: 1, interior: X2([Parachain(1001), AccountId32 { network: None, id: [150, 141, 187, 98, 102, 33, 87, 174, 108, 105, 38, 201, 33, 252, 99, 215, 105, 11, 253, 230, 89, 13, 87, 138, 18, 41, 154, 220, 108, 179, 239, 229] }]) }), message_id: [207, 224, 150, 20, 104, 44, 231, 40, 113, 150, 204, 227, 199, 74, 162, 200, 138, 143, 3, 176, 227, 143, 46, 145, 94, 71, 215, 205, 7, 177, 227, 162], topic: Some([207, 224, 150, 20, 104, 44, 231, 40, 113, 150, 204, 227, 199, 74, 162, 200, 138, 143, 3, 176, 227, 143, 46, 145, 94, 71, 215, 205, 7, 177, 227, 162]) } available_weight=Weight { ref_time: 43793552000, proof_size: 387172 } available_amount=12905733320
2025-10-04T07:13:08.329802Z TRACE xcm::weight: UsingComponents::refund_weight amount=12249999988
2025-10-04T07:13:08.329816Z TRACE xcm::ensure_can_subsume_assets: Ensuring subsume assets work worst_case_holding_len=1 holding_limit=64
2025-10-04T07:13:08.329843Z TRACE xcm::refund_surplus: total_refunded=Weight { ref_time: 41967000000, proof_size: 367500 }
2025-10-04T07:13:08.332568Z TRACE xcm::post_process: Trapping assets in holding register holding_register=AssetsInHolding { fungible: {AssetId(Location { parents: 1, interior: Here }): 12249999988}, non_fungible: {} } context=XcmContext { origin: Some(Location { parents: 1, interior: X2([Parachain(1001), AccountId32 { network: None, id: [150, 141, 187, 98, 102, 33, 87, 174, 108, 105, 38, 201, 33, 252, 99, 215, 105, 11, 253, 230, 89, 13, 87, 138, 18, 41, 154, 220, 108, 179, 239, 229] }]) }), message_id: [207, 224, 150, 20, 104, 44, 231, 40, 113, 150, 204, 227, 199, 74, 162, 200, 138, 143, 3, 176, 227, 143, 46, 145, 94, 71, 215, 205, 7, 177, 227, 162], topic: Some([207, 224, 150, 20, 104, 44, 231, 40, 113, 150, 204, 227, 199, 74, 162, 200, 138, 143, 3, 176, 227, 143, 46, 145, 94, 71, 215, 205, 7, 177, 227, 162]) } original_origin=Location { parents: 1, interior: X1([Parachain(1001)]) }
2025-10-04T07:13:08.333144Z TRACE xcm::post_process: Execution failed instruction=0 error=NotHoldingFees original_origin=Location { parents: 1, interior: X1([Parachain(1001)]) }
2025-10-04T07:13:08.333167Z TRACE xcm::process-message: XCM message execution incomplete error=NotHoldingFees index=0 used=Weight { ref_time: 1826552000, proof_size: 19672 }There was a problem hiding this comment.
iiuc, are we talking about this XCM: https://github.com/polkadot-fellows/runtimes/blob/main/system-parachains/encointer/src/treasuries_xcm_payout.rs#L267-L283? Maybe just for completeness, could you add the full XCM from the logs here?
Do you have a block/transaction, that we can replay and investigate more?
From the logs above, I don't see any errored instruction after SetAppendix, which is just TransferAsset, so it passed.
But the SetAppendix(ReportError(..) is trying to send self.error register, even if the error register is None.
There are couple of issues or things to think about:
ReportErrorsends/reports error register regardless if Some/None, which means, for every successful XCM processing, it send one everytimeXcm(ReportError(None), which is kind of "questionable":
- ReportError is just reporting the error register
- but actually here, the intention or expected usage is more like "ReportError only if Some(error)", which the ReportError actuall does not support
- potential dirty hack/fix looks like to wrap ReportError processing with
if let Some(error) = self.error {...}here: https://github.com/paritytech/polkadot-sdk/blob/master/polkadot/xcm/xcm-executor/src/lib.rs#L1141-L1151, but it is not aligned with actual ReportError description:Immediately report the contents of the Error Register to the given destination via XCM.https://github.com/paritytech/polkadot-sdk/blob/d36d48a25c80502c577df3b1a88eadb3b3d3b6be/polkadot/xcm/src/v5/mod.rs#L588-L597 (sure, we could potentially change the description and add the IF) - but maybe cleaner solution would be to add another instruction ReportErrorIfPresent or something
- potential dirty hack/fix looks like to wrap ReportError processing with
- yes, I think that
NotHoldingFeesis correct error, because we can seeWithdrawAsset(remote_fee),PayFees(remote_fee)and here I am little bit confused, becausePayFeestakes the wholeremote_feefrom theself.holding, soself.holdingis now empty, and theunspentfrom trader should be return back toself.fees, but looks like it is not at this point
- so the error
Holding doesn't hold enough for feesis from the wrong branch: https://github.com/paritytech/polkadot-sdk/blob/d36d48a25c80502c577df3b1a88eadb3b3d3b6be/polkadot/xcm/xcm-executor/src/lib.rs#L612-L614, I would expect here thatself.feescontains at leastself.fees.subsume_assets(unspent);, but it is empty, this is what I don't understand now, needs (clean morning mind) more investigation - and interesting is that
refund_weightwhich adds something back to the holding is executed after all this stuff fromvm.post_process
cc: @franciscoaguirre @raymondkfcheung
@clangenb Hmm, I still had it in my head that somewhere during PR reviews I came across ReportError, and I think it was exactly this XCM :D #679 (comment)
There was a problem hiding this comment.
Other thing is if DescendOrigin (as effective origin) should pay for ReportError or destination sovereign account (which would be Encointer parachain), but this needs more thinking, not to open possible free spamming with ReportError or something, so something like:
SetAppendix(Xcm(vec![
// Refund everything to `effective origin)`
RefundSurplus,
DepositAsset { assets: AssetFilter::Wild(WildAsset::All), beneficiary: from_at_target },
// Change origin to Encointer location
AliasOrigin(encointer_location as Location(1/0, Parachain(1001)),
// Encointer is waived for fees, so no fees charged for `self.send(`
ReportError(QueryResponseInfo {
destination: destination.clone(),
query_id,
max_weight: Weight::zero(),
}),
])),
There was a problem hiding this comment.
I agree with @bkontur that AliasOrigin + reordering ReportError looks like the right fix here. We just need to make sure it can't be abused for free ReportError spam.
For tracing, I'll follow up with targeted logs around the appendix path. I need a bit more time to place these in the right spots so they're useful without being noisy.
There was a problem hiding this comment.
hmm, I am also not sure if AliasOrigin would work here, because of Aliasers:
target does not start with origin (AliasChildLocation):
origin: Location { parents: 1, interior: X2([Parachain(1001), AccountId32{..}
target: Location { parents: 1, interior: X1([Parachain(1001))
For:
pub struct AliasChildLocation;
impl ContainsPair<Location, Location> for AliasChildLocation {
fn contains(origin: &Location, target: &Location) -> bool {
return target.starts_with(origin)
}
}
There was a problem hiding this comment.
Cool, thanks for looking into this.
iiuc, are we talking about this XCM: https://github.com/polkadot-fellows/runtimes/blob/main/system-parachains/encointer/src/treasuries_xcm_payout.rs#L267-L283? Maybe just for completeness, could you add the full XCM from the logs here?
You are right @bkontur, this is the xcm we are talking about.
In this setup at least, I believe that it could not be abused for free error spams because whatever is triggering the alias back into the Encointer location would need to pay XCM execution fees on AH. So they just get the spam for free, but not without paying for the enclosing XCM first, which I guess is a reasonable spam prevention mechanism.
Regardless, in order to have encointer fixed soon, I suggest that I open another issue because I think that fixing this properly might even need some design thought in XCM first. We would really like to have a working patch released.
I do not have a block to replay, but I could give you a chopsticks setup that could trigger the XCM from encointer to AH with one single extrinsic. Would this work @bkontur?
@clangenb Hmm, I still had it in my head that somewhere during PR reviews I came across ReportError, and I think it was exactly this XCM :D #679 (comment)
Haha, you are on point ;)
There was a problem hiding this comment.
Regarding your second point @bkontur, I have opened an issue for further investigation, as this might be a general XCM bug: paritytech/polkadot-sdk#10078.
acatangiu
left a comment
There was a problem hiding this comment.
We can move the ReportError enhancements discussion to its own thread.
Approving this PR which removes the malfunctioning error reporting.
|
/merge |
d4859bc
into
polkadot-fellows:main
|
Enabled Available commands
For more information see the documentation |
Even though we have verified that asset payment on Asset Hub works in general, we have now encointered a protocol bug in the encointer pallets erroneously registering asset payouts as native payouts. The patch bump of the encointer-pallets fixes this.
Additionally, while investigating above issue, we found that there is an XCM error. While our assets are indeed paid out on asset hub, the overall XCM result is a
NotHoldingFeeserror, which came from theReportErrorinstruction from the appendix. While this needs some more investigation, we just removed it, as encointer does not use it for now. The integration tests have been enhanced to also check successful XCM execution on Asset Hub.