search

paritytech / polkadot-sdk

The Parity Polkadot Blockchain SDK
https://polkadot.network/
1.77k stars 634 forks source link

SetAppendix(ReportError(QueryResponseInfo)) instruction stopped reporting xcm execution result #3198

Open GopherJ opened 7 months ago

GopherJ commented 7 months ago

Is there an existing issue?

Experiencing problems? Have you tried our Stack Exchange first?

Description of bug

  1. reserve_transfer_assets may causes users to lose all funds if insufficient KSM/DOT left in wallet.
  2. SetAppendix(ReportError(QueryResponseInfo)) seems stopped reporting xcm status to parachain. We start to see this after the latest kusama upgrade

Steps to reproduce

No response

dudo50 commented 7 months ago

Hey @GopherJ , I have already reported this here: https://github.com/paritytech/polkadot-sdk/issues/3050 The fix is being issued in the latest 1.1.2 update and you can vote on it here: https://kusama.polkassembly.io/referenda/339

acatangiu commented 7 months ago

Marking as duplicate of https://github.com/paritytech/polkadot-sdk/issues/3050

GopherJ commented 7 months ago

@acatangiu @dudo50 How about another issue?

SetAppendix(ReportError(QueryResponseInfo)) seems stopped reporting xcm status to parachain. We start to see this after the latest kusama upgrade

I think it should be re-opened

acatangiu commented 7 months ago

@franciscoaguirre can you take a look please?

GopherJ commented 7 months ago

hi @franciscoaguirre do you have any guess? we are live on kusama as Parallel Heiko network, we had this issue since 13 days ago

You can see kusama is not re-calling our notification_received method since that time https://parallel-heiko.subscan.io/event?page=1&time_dimension=date&module=liquidstaking&event_id=notificationreceived

code ref: https://github.com/parallel-finance/parallel/blob/efa2146c42fd85743a6565b32a9bb65e9e991620/pallets/xcm-helper/src/lib.rs#L258-L280

I've checked the transact was executed successfully on kusama but without callback

franciscoaguirre commented 7 months ago

I'll look into it and let you know what I find

franciscoaguirre commented 7 months ago

I think I know what's the problem. @GopherJ can you provide me with the extrinsic that triggers the notification? So I can replay it locally and look at the logs

GopherJ commented 7 months ago

@franciscoaguirre sure

You can check https://parallel-heiko.subscan.io/event?page=1&time_dimension=date&module=liquidstaking&event_id=unbonding these are all xcm extrinsics that we sent to kusama

picking https://parallel-heiko.subscan.io/block/5346019?tab=event&event=5346019-3, it comes from the block 0x7136d6bab11772edc763e98479f851aad8e0b979166fc8231d46d21b4a78a354 which corresponds relaychian block: 21,766,456

and it finally executed in relaychain block 21,766,459 as you can see from: https://polkadot.js.org/apps/?rpc=wss%3A%2F%2Fkusama.api.onfinality.io%2Fpublic-ws#/explorer/query/21766459

GopherJ commented 7 months ago

The extrinsic sent to kusama should be simple, it should be staking.unbond(31204272643408)

GopherJ commented 7 months ago

@franciscoaguirre hi, do you have any ideas?

franciscoaguirre commented 7 months ago

I haven't managed to run it locally with the same configuration, but it's an issue with the ReportError instruction needing to send a new message and not having anything in the holding register to pay for delivery fees.

I saw the message before the appendix does RefundSurplus and DepositAsset. Could you try removing the DepositAsset and putting it after the ReportError instruction in the appendix? That would make sure the weight surplus is still in the holding register, so it can be used for the delivery fees.

This situation definitely needs to be improved, but please let me know if that solves the issue.

GopherJ commented 7 months ago

@acatangiu could you reopen this issue?

GopherJ commented 6 months ago

@franciscoaguirre @acatangiu shall this be fixed on kusama side instead, ideally we shouldn't see breaking change going this way. How do you think?

Aleksey4Crypro commented 6 months ago

Hey guys, how much time approximately remains for you to fix the issue caused by KSM update? @franciscoaguirre @acatangiu

franciscoaguirre commented 6 months ago

It is going to be fixed in the Kusama side, maybe I didn't make myself clear. What I meant is while I'm working on a fix, you can try what I told you.

Once I have the fix I'll push for getting it on Kusama as soon as possible.

acatangiu commented 6 months ago

Hey guys, how much time approximately remains for you to fix the issue caused by KSM update? @franciscoaguirre @acatangiu

If you have any workaround (like the one suggested by Francisco), I suggest you use it. Even if we'd have a fix for this tomorrow, it will still take a few weeks until the next https://github.com/polkadot-fellows/runtimes/ release containing it, plus around 2 weeks until on-chain upgrade referenda passes and is enacted.

TLDR: under the normal release process it will take a few weeks at least..

franciscoaguirre commented 6 months ago

There should be no problem if you don't empty the holding register (do RefundSurplus) before the ReportError instruction.

GopherJ commented 6 months ago

@franciscoaguirre @acatangiu why has this version been upgraded to polkadot before the fix? it's ridiculous

franciscoaguirre commented 6 months ago

Hi, unfortunately getting to the fix took a while, since we want to make sure we don't introduce new errors.

Have you tried making sure you don't deposit until the very end of the program execution? In the appendix. I know it's not ideal, but it should help you continue your operations.

The fix is being worked on here: https://github.com/paritytech/polkadot-sdk/pull/3450

franciscoaguirre commented 6 months ago

Also, I don't see a way for you to not have to change anything and have this work sadly. In an ideal world, it would just be a simple fix that wouldn't require any "breaking" change from your side.

acatangiu commented 6 months ago

@franciscoaguirre @acatangiu why has this version been upgraded to polkadot before the fix? it's ridiculous

I guess it's a mix of things:

  1. failure on our side to raise visibility of this issue in governance proposal to upgrade Polkadot
  2. failure on your side to raise visibility of this issue in governance proposal to upgrade Polkadot
  3. the fact that this Polkadot release came with a many awaited features/improvements and the community was happy to take it as is

Regarding point 1 above, a workaround was proposed to you for which you did not provide any feedback or answer on. For better alignment on priority/impact/importance, this two-way communication needs improvement.

franciscoaguirre commented 6 months ago

@GopherJ We are discussing a solution to this problem in XCM RFC 53, since it needs a change to the format itself. All contributions are welcome.

KudosDot commented 6 months ago

@franciscoaguirre any updates on this?

your help is much appreciated.

acatangiu commented 6 months ago

@GopherJ @KudosDot asking yet again, have you tried the following workaround: https://github.com/paritytech/polkadot-sdk/issues/3198#issuecomment-1935071213 ?

Let me clarify the situation here: the existing XCMs that you were using are probably forever broken, there is no "fix" for them AFAIK.

The reason of the breakage is a security fix which enforces that all XCM message deliveries have to be paid. Aka free message delivery is a thing of the past. This ^ was absolutely required to close an attack surface where the whole network was potentially DOSable.

The side-effects of this fix were a number of unexpected and unintended breakages of existing XCM programs out there.

Unfortunately, some of them can't be fixed without changing them - e.g. if an XCM program relies on free delivery, it will never work anymore, the XCM program has to change, not the system underneath it.

Please take the above into consideration, reassess your issue accordingly, and let us know if:

  1. you believe your XCM program as is should work and it is an underlying system problem/bug (also please provide reasoning) or,
  2. you can adapt your XCM program to work under the new system conditions
Aleksey4Crypro commented 6 months ago

Hey guys, i have a few questions:

1) How much time remains to fix issues with the XCM? This issue is already for more than 3 weeks. 2) Why CMs of Parity team saying that Polkadot doesn't have any issues and this issue from Parallel Side? Can you ask your CMs to provide to the users the correct information, since it is not the issue from Parallel side and we can't influence on it. Right now, a lot of users are writing that we don't want to fix this issue because your CMs are providing the wrong information.

Thanks in advance for your replies.

@franciscoaguirre @acatangiu @dudo50

pr0gress0r commented 6 months ago

Hi everyone,

I am an investor in Parallel Finance, and due to this issue, we have been unable to unstake our DOT/KSM for a month now. All we get when asking when the issue will be resolved is "Parity is working on the fix," but there is no information on timelines. Reading into this matter https://github.com/paritytech/polkadot-sdk/issues/3198#issuecomment-1991724410, I understand that Parity will not be making the fix and the Parallel team should address the issue. If that's the case, please help them do so, as they believe you should solve this problem on your own. We have been unable to access our funds for a month, so any options that could help us would be appreciated. Thank you in advance.

@acatangiu @franciscoaguirre @dudo50 @ggwpez

dudo50 commented 6 months ago

Unsure why I am getting tagged guys, I don't work at Parity. I was reporting issue regarding asset loss during XCM.

Thanks for your understanding.

With kind regards, Dudo

franciscoaguirre commented 6 months ago

Hello @pr0gress0r, I'm very sorry to hear that.

There's a short-term and a long-term fix for this issue.

The short-term fix requires some work on their end. We've already said what works needs to be done. We are happy to give as many pointers as needed. We've asked if they've tried the changes and encountered any issue, but haven't gotten a response.

The long-term fix is being worked on. The reason there's no timeline update is that our main priority is to make sure nothing is broken again, as this was very unfortunate, and we don't want to make things more difficult for other teams as well.

@GopherJ I'd be happy to work alongside you to implement the fix we proposed.

pr0gress0r commented 5 months ago

Hi everyone,

@Aleksey4Crypro @GopherJ any updates? Please reply to @franciscoaguirre if you have problems implementing the fix, he will help resolve the issue.

oilreg commented 5 months ago

Parallel is kinda in denial regarding this .. same as @pr0gress0r my funds are locked in DOT / KSM liquid staking and also the redeeming of the crowdloans doesnt work, atm i am also worried with the staking rewards if we might get slashed, cause on their website the amount of sDOT / sKSM stays the same

.. posting a link to this discussion on their discord autodeletes the post with the link .. so it seems the only happy ending to this dilemma is a fix from parity

oilreg commented 5 months ago

wanted to give a little update on my end... as of today .. unstaking sKSM with the 7 days waiting period seemed to work.. in 7 days i know more