paritytech / polkadot-sdk

The Parity Polkadot Blockchain SDK
https://polkadot.com/
1.88k stars 688 forks source link

`claim_assets` doesn't work if the trapped asset can't be used to pay for execution fees. #4433

Closed franciscoaguirre closed 2 weeks ago

franciscoaguirre commented 6 months ago

The claim_assets extrinsics claims in the simplest way possible, it just grabs the asset, uses it to pay execution fees, and deposits it on the beneficiary account. This assumes the asset can be used to pay for fees, which might not be the case, i.e. an NFT or a fungible token without a pool.

The extrinsic creates the following program:

It could be fixed by changing that to the following program:

We could probably pass in a fees parameter that could be None to use the same assets for fees. Sadly, this would be a breaking change. We could add a new extrinsic claim_assets_with (or claim_not_sufficient_assets) that takes this fees parameter (not necessarily an option since we already have the other extrinsic).

dogwifdots commented 5 months ago

Woof, just found this while researching for an upcoming token claimable airdrop for DOT holders that Dog wif dots (WIFD) is planning. Could I use this function for an embedded extrinsic function on our website to web3 login and click a claim your WIFD button to start the TX signature in the users wallet?

Many thanks in advance, Dog wif dots dev

franciscoaguirre commented 5 months ago

@dogwifdots This extrinsic is for claiming assets trapped during XCM execution. It's useful to get back your assets when an error occured. It's not for airdrops.

lolmcshizz commented 5 months ago

Hey guys - I believe that this will resolve an issue we have with users and their trapped DED tokens. We are currently unable to claim for them because DED can't be used to pay for the execution - here are some example transactions that are currently trapped:

https://hydration.subscan.io/extrinsic/4783346-2?tab=xcm_transfer

https://assethub-polkadot.subscan.io/xcm_message/polkadot-bda8b962fd0fac0f8e977138929c402ea4fa85df

https://hydradx.subscan.io/extrinsic/4783421-2?tab=xcm_transfer

https://hydradx.subscan.io/extrinsic/0x1b82ad38c61a9ccf64d1929964080b81de8a9d541f4b243b5a2e77bdf5c7071c?tab=xcm_transfer

https://hydradx.subscan.io/extrinsic/4783421-2?tab=xcm_transfer

franciscoaguirre commented 5 months ago

So, what this issue says is actually incorrect. The claim_assets extrinsic is not using BuyExecution, it just uses the extrinsic fees for execution. This means it will never fail because of fees.

So @lolmcshizz this will not solve your issue.

Those trapped assets can be claimed but only by the origin of the chain. This will be improved in the future but sadly that's what we have now. So you can create a governance proposal to claim them and return them to their respective owners.

The way of doing it would be to send an XCM to AssetHub from the root origin. This XCM would roughly look like this:

WithdrawAsset
BuyExecution // <- To pay for fees
ClaimAsset // <- Claim the actual trapped assets
DepositAsset // <- Deposit to the original destination which failed before

You would then need to do this ClaimAsset -> DepositAsset for each user you want to return the trapped assets too.

You could also do a batch with many small XCMs.

Another option is to send an XCM that calls the claim_assets extrinsic remotely:

WithdrawAsset
BuyExecution
Transact { claim_assets }
franciscoaguirre commented 5 months ago

Closing since this is not how the claim_assets extrinsic works 😅

green-jay commented 5 months ago

Hi @franciscoaguirre, unfortunately none of the suggested options seem to work.

Example of trapped assets: https://assethub-polkadot.subscan.io/extrinsic/5950189-0?event=5950189-5

WithdrawAsset
BuyExecution // <- To pay for fees
ClaimAsset // <- Claim the actual trapped assets
DepositAsset // <- Deposit to the original destination which failed before

Got polkadotXcm.SendFailure on Hydra (likely indicating wrong construction of the message) Calldata on Hydra (root origin): 0x6b0003010100a10f03100004000002043205011f0002093d0013000002043205011f0002093d0000180800000204320578000f6fcf1d719ea501000002043205011f00d19200000d010000010100d43593c715fdd31c61141abd04a99fd6822c8558854ccde39a5684e7a56da27d

WithdrawAsset
BuyExecution
Transact { claim_assets }

Got assets trapped on AH (xcm success: no) Calldata on Hydra (root origin): 0x6b0003010100a10f030c0004000002043205011f0002093d0013000002043205011f0002093d0000060102286beeaa11040009011f0c0408000204320578000f6fcf1d719ea5010002043205011f00d1920400010100d43593c715fdd31c61141abd04a99fd6822c8558854ccde39a5684e7a56da27d

franciscoaguirre commented 4 months ago

Hello @green-jay, I tried the first extrinsic on chopsticks (0x6b0003010100a10f03100004000002043205011f0002093d0013000002043205011f0002093d0000180800000204320578000f6fcf1d719ea501000002043205011f00d19200000d010000010100d43593c715fdd31c61141abd04a99fd6822c8558854ccde39a5684e7a56da27d) and got the same polkadotXcm.SendFailure error. By changing the SafeXcmVersion to 3, I got it to work. Please let me know if that works.

green-jay commented 4 months ago

hi @franciscoaguirre,

It helped to send the message successfully but the claim on AH still wasn't successful

Juanma0x commented 4 months ago

@green-jay, if you wish to collect similar events to propose a common referendum on Hydration for claiming assets, here’s an event shared with us by an affected user:

https://hydration.subscan.io/xcm_message/polkadot-a9f9dabbc0fd2066b673a2ab0611fad88dc2e699

franciscoaguirre commented 4 months ago

@green-jay I managed to make it work locally with chopsticks.

For testing locally, I:

The thing I needed to change was increase the fees from 1_000_000 to 10_000_000 (less might be possible). After that I managed to claim the funds and deposit them to another account.

green-jay commented 4 months ago

@franciscoaguirre Why were the first two steps necessary? Chopsticks makes a local copy of the actual storage so the trapped assets should be already there? As well as 7,5M USDT on the sovereign account. Also the processing of the message shouldn't cost more than 1 USDT. To make sure I tried with 10 USDT but the processing failed again. Did you manage to claim any existing trapped assets?

franciscoaguirre commented 4 months ago

@green-jay Hmm, you're right, maybe I was using an old block or I just jumped the gun and added the storage before I saw it wasn't an issue.

I agree processing shouldn't cost more than 1 USDT, but I did manage to claim the assets. Can you please share the errors you're getting?

green-jay commented 4 months ago

@franciscoaguirre Yeah XCM logs would definitely help. I added runtime-log-level: 5 to chopsticks chain config but for some reason it does not increase log level for Asset Hub. I think I might need AH wasm that is build with some debug flags and override it, would you have that by any chance?

franciscoaguirre commented 4 months ago

@green-jay You do need the wasm since the wasm for the live runtimes is built without logs to reduce its size. You can find it in the releases page: https://github.com/polkadot-fellows/runtimes/releases/tag/v1.2.8.

Alternatively, you can always build it by going to the polkadot-fellows/runtimes repo and running cargo build --release -p asset-hub-polkadot-runtime then using the wasm under target/release/wbuild/asset-hub-polkadot-runtime/asset-hub-polkadot-runtime.compact.compressed.wasm.

green-jay commented 4 months ago

@franciscoaguirre release wasm didn't produce the log but after i built it without the --release flag and used debug wasm, logs are finally there :) ah_xcm_log.txt

We upgraded to safeXcmVersion 3. The error i'm getting using both extrinsics above is: xcm::execute TRACE: !!! ERROR: UnknownClaim despite the claim looks exactly like what was trapped in the event: https://assethub-polkadot.subscan.io/extrinsic/5950189-0?event=5950189-5

franciscoaguirre commented 4 months ago

@green-jay Weird that the claim didn't exist. I did load the trapped funds into storage, which probably is the only difference that explains why it worked for me.

You could check storage before running the extrinsic and see if it's the same as on the live network

green-jay commented 4 months ago

@franciscoaguirre Yes, it is the same and i have been testing claiming in the past in Chopsticks so this should work. Maybe you could share more details about loading assets into the storage (did you use dev_setStorage or sth else?) How did the events look like after the successful claim? Did you see assets.issued for asset with id 30? Could you please try to test without injecting it?

franciscoaguirre commented 4 months ago

I'm using this extrinsic and the claim works: 0xf0006b0003010100a10f03100004000002043205011f00025a620213000002043205011f00025a620200180800000204320578000f6fcf1d719ea501000002043205011f00d19200000d010000010100d43593c715fdd31c61141abd04a99fd6822c8558854ccde39a5684e7a56da27d

This is without putting anything in storage, you were right I didn't have to do that.

I get the following events:

Screenshot 2024-07-08 at 23 38 37

I don't know why I got a TooExpensive error and needed to increase the fees and why you got an UnknownClaim error. Still trying to figure that out.

green-jay commented 4 months ago

@franciscoaguirre there has to be some difference in your setup from the live chain that allows this - is there anything beside sudo pallet? i asked other team member to simulate the tx and he also got the UnknownClaim error

green-jay commented 3 months ago

@franciscoaguirre We need to figure out whether the above is a real issue and in case it is solve it in xcmv5.

Scenario: Someone sends two tokens in one xcm message, one is fee sufficient, one fee insufficient. The sufficient asset amount is too low to cover fee to process the message further and insufficient can't be used for fees. The funds then seem stuck as cannot be claimed as the holding can't be topped up. The fee insufficient asset can represent a large $ value.

franciscoaguirre commented 3 months ago

@franciscoaguirre We need to figure out whether the above is a real issue and in case it is solve it in xcmv5.

Scenario: Someone sends two tokens in one xcm message, one is fee sufficient, one fee insufficient. The sufficient asset amount is too low to cover fee to process the message further and insufficient can't be used for fees. The funds then seem stuck as cannot be claimed as the holding can't be topped up. The fee insufficient asset can represent a large $ value.

Even if the amount of the asset that can pay for fees is too low, you can always withdraw some assets for fees first. That is what's happening in your extrinsic, you first WithdrawAsset to get some assets to pay for fees, then you BuyExecution to pay for said fees, you ClaimAsset and then DepositAsset. Since the assets inside ClaimAsset are not used to pay for fees, the scenario you're describing is not a problem.

franciscoaguirre commented 3 months ago

@green-jay

My polkadot-asset-hub config file: https://pastebin.com/iFCpDMAS My hydradx config file: https://pastebin.com/Jd4TQXBd Chopsticks logs: https://pastebin.com/9CmXWWh9

This is with the extrinsic you sent me but running it with sudo. See how I get a TooExpensive error. I never got the UnknownClaim error you are getting. Can you share your config files and your logs?

mt-gareth commented 3 months ago

@franciscoaguirre I am looking for some guidance on this XCM message that is much like what Green-Jay said. USDT and DED were sent from HydraDX to AssetHub, the USDT was not enough and both got trapped. How is it possible to claim the DED that was sent?

https://hydration.subscan.io/xcm_message/polkadot-bda8b962fd0fac0f8e977138929c402ea4fa85df

green-jay commented 3 months ago

@franciscoaguirre release wasm didn't produce the log but after i built it without the --release flag and used debug wasm, logs are finally there :) ah_xcm_log.txt

We upgraded to safeXcmVersion 3. The error i'm getting using both extrinsics above is: xcm::execute TRACE: !!! ERROR: UnknownClaim despite the claim looks exactly like what was trapped in the event: https://assethub-polkadot.subscan.io/extrinsic/5950189-0?event=5950189-5

Hey @franciscoaguirre, i provided the log above. I see you are using overriden wasms, that can cause potentially different behaviour. This is the script i use to execute a referendum:

await api.rpc('dev_setStorage', {
 scheduler: {
   agenda: [
     [
       [number + 1], [
         {
           call: {
             Inline: '0x030300016d6f646c70792f7472737279000000000000000000000000000000000000000052000000000013000064a7b3b6e00d010200c91f01006d6f646c6f6d6e69706f6f6c000000000000000000000000000000000000000000'
           },
           origin: {
             system: 'Root'
           }
         }
       ]
     ],
     [
       [number + 2], [
         {
           call: {
             Lookup: {
               hash: '0x1a77fe92f5a374ea9f4d4056b591dd2ab7e6886bb9d0b88d5cafe61029502d8d',
               len: 1423658
             }
           },
           origin: {
             system: 'Root'
           }
         }
       ]
     ]
   ]
 }
})
await api.rpc('dev_newBlock', { count: 2})

I'm using plain on-chain Hydration wasm and debug built plain AH wasm (just to see the log), nothing special there: hydration.txt ah.txt

mt-gareth commented 2 months ago

@franciscoaguirre would you be able to take a look at this? I am having issues with trapped assets and when I ask for help they point me here saying when this is fixed I would be able to claim them. My trapped transaction is above.

franciscoaguirre commented 1 month ago

Hello @mt-gareth. I see you tried to claim your assets multiple times. The first ones were not built correctly, but the two last ones were. The reason they failed is because the origin of the trapped funds is the chain itself, Hydration. This means that the chain itself has to send a message to Asset Hub to claim your funds and give them to you.

That origin is the chain and not your account sadly because of an issue in the interaction between the asset trapping mechanism and reserve asset transfers. That will be fixed once XCMv5 comes out at the end of the year and after that all trapped assets resulting from transfers will be able to be claimed by the originating user account.

The thing that needs to happen is a governance proposal from Hydration to send a message with the root priviledges to Asset Hub, claim your funds and give them to you. I'm happy to work on creating a proposal to return your funds.

mt-gareth commented 1 month ago

@franciscoaguirre Thank you so much for looking into it, this was well beyond me, and I have been trying for months, thank you.

Yes any help you can provide in getting these funds unlocked would be greatly appreciated. Im not sure of the process and I can take anything to Hydration that needs to be done there. There support has been responsive but they did not know how to help.

franciscoaguirre commented 1 month ago

Building the right message and putting it on Hydration's governance will work. I can work alongside them to build the correct one.

green-jay commented 1 month ago

@franciscoaguirre I just re-tested it and the message still fails with UnknownClaim, you can use the script I posted above to verify it

franciscoaguirre commented 4 weeks ago

@green-jay I don't seem to be able to decode the call you shared.

0x030300016d6f646c70792f7472737279000000000000000000000000000000000000000052000000000013000064a7b3b6e00d010200c91f01006d6f646c6f6d6e69706f6f6c000000000000000000000000000000000000000000

I tried on both hydration and asset hub using the chopsticks config files you shared.

green-jay commented 4 weeks ago

@franciscoaguirre It was just an example script. The calldata, hash and length in the script have to be replaced by the actual call, so in our case it was

await api.rpc('dev_setStorage', {
 scheduler: {
   agenda: [
     [
       [number + 1], [
         {
           call: {
             Inline: '0x6b0003010100a10f03100004000002043205011f0002093d0013000002043205011f0002093d0000180800000204320578000f6fcf1d719ea501000002043205011f00d19200000d010000010100d43593c715fdd31c61141abd04a99fd6822c8558854ccde39a5684e7a56da27d'
           },
           origin: {
             system: 'Root'
           }
         }
       ]
     ],
     [
       [number + 2], [
         {
           call: {
             Lookup: {
               hash: '0xa7a74ea30c7daa1332001b2dfbc36c71fcbd23fc4f815683a4399dea4fa240cf',
               len: 110
             }
           },
           origin: {
             system: 'Root'
           }
         }
       ]
     ]
   ]
 }
})
franciscoaguirre commented 3 weeks ago

The call you sent fails with UnknownClaim because it uses ClaimAsset with a ticket of Here. The ticket is used to provide the version of the trapped assets. Here means latest which won't work, you need to put { parents: 0, interior: X1(GeneralIndex(3)) } to convey version 3. I changed that and it worked, attaching the logs.

asset-hub-logs.txt

green-jay commented 3 weeks ago

Aaah, that was it! Did you find that info about the ticket in some docs or you had to dig into code? I found only this in the xcm-format docs: ticket: Location: The ticket of the asset; this is an abstract identifier to help locate the asset This never mentioned version, just the location. It also explains why your artificial claim worked, because you probably created the trap as v4.

Thanks a lot for your help @franciscoaguirre!

franciscoaguirre commented 3 weeks ago

You're welcome. I had to look up the code, the idea of the ticket is that it's abstract and it could be used differently in different implementations, at least that's why I think it was done that way. Pallet-xcm is the one to look out for to know how to use it.

Hope with these the proposals can be created, if you need any help let me know

franciscoaguirre commented 2 weeks ago

@green-jay Closing the issue, let me know if it should be reopened