Remove NotConcrete error #3867

chungquantin · 2024-03-27T16:34:25Z

Description

Link to issue: remove NotConcrete error #3651

polkadot address: 19nSqFQorfF2HxD3oBzWM3oCh4SaCRKWt1yvmgaPYGCo71J

polkadot/xcm/xcm-executor/src/traits/asset_transfer.rs

prdoc/pr_3867.prdoc

polkadot/xcm/xcm-executor/src/traits/asset_transfer.rs

- prdoc is not essential for a cleanup issue - Remove `InvalidAssetNonConcrete` type because this is unused if there is no `NotConcrete` type in asset transfer.

polkadot/xcm/xcm-executor/src/traits/asset_transfer.rs

franciscoaguirre

👍 Should be good to merge after CI passes

chungquantin · 2024-03-30T03:15:35Z

@franciscoaguirre the integration tests keeps failing at transaction_recrusion_limit_works() and I saw it logged stack limit error. Guess it is not related to the changes in this request right?

running 1 test
test transact_recursion_limit_works ... FAILED
failures:
failures:
    transact_recursion_limit_works
test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 4 filtered out; finished in 2.18s
--- TRY 6 STDERR:        xcm-executor-integration-tests transact_recursion_limit_works ---
Mar 30 02:18:31.373 ERROR xcm::pallet_xcm::execute: XCM execution failed with error ExceedsStackLimit    
Mar 30 02:18:31.373 ERROR xcm::pallet_xcm::execute: XCM execution failed with error ExpectationFalse    
Mar 30 02:18:31.373 ERROR xcm::pallet_xcm::execute: XCM execution failed with error ExpectationFalse    
Mar 30 02:18:31.373 ERROR xcm::pallet_xcm::execute: XCM execution failed with error ExpectationFalse    
Mar 30 02:18:31.373 ERROR xcm::pallet_xcm::execute: XCM execution failed with error ExpectationFalse    
Mar 30 02:18:31.373 ERROR xcm::pallet_xcm::execute: XCM execution failed with error ExpectationFalse    
Mar 30 02:18:31.373 ERROR xcm::pallet_xcm::execute: XCM execution failed with error ExpectationFalse    
Mar 30 02:18:31.373 ERROR xcm::pallet_xcm::execute: XCM execution failed with error ExpectationFalse    
Mar 30 02:18:31.373 ERROR xcm::pallet_xcm::execute: XCM execution failed with error ExpectationFalse    
Mar 30 02:18:31.373 ERROR xcm::pallet_xcm::execute: XCM execution failed with error ExpectationFalse    
Mar 30 02:18:31.374 ERROR xcm::pallet_xcm::execute: XCM execution failed with error ExpectationFalse    
Mar 30 02:18:31.375 ERROR xcm::pallet_xcm::execute: XCM execution failed with error ExceedsStackLimit    
Mar 30 02:18:31.375 ERROR xcm::pallet_xcm::execute: XCM execution failed with error ExpectationFalse    
Mar 30 02:18:31.375 ERROR xcm::pallet_xcm::execute: XCM execution failed with error ExpectationFalse    
Mar 30 02:18:31.375 ERROR xcm::pallet_xcm::execute: XCM execution failed with error ExpectationFalse    
Mar 30 02:18:31.375 ERROR xcm::pallet_xcm::execute: XCM execution failed with error ExpectationFalse    
Mar 30 02:18:31.375 ERROR xcm::pallet_xcm::execute: XCM execution failed with error ExpectationFalse    
Mar 30 02:18:31.375 ERROR xcm::pallet_xcm::execute: XCM execution failed with error ExpectationFalse    
Mar 30 02:18:31.375 ERROR xcm::pallet_xcm::execute: XCM execution failed with error ExpectationFalse    
Mar 30 02:18:31.376 ERROR xcm::pallet_xcm::execute: XCM execution failed with error ExpectationFalse    
Mar 30 02:18:31.376 ERROR xcm::pallet_xcm::execute: XCM execution failed with error ExpectationFalse    
Mar 30 02:18:31.376 ERROR xcm::pallet_xcm::execute: XCM execution failed with error ExpectationFalse    
thread 'transact_recursion_limit_works' panicked at polkadot/xcm/xcm-executor/integration-tests/src/lib.rs:146:9:
assertion `left == right` failed
  left: 0
 right: 10
stack backtrace:
   0: rust_begin_unwind
             at /rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library/std/src/panicking.rs:645:5
   1: core::panicking::panic_fmt
             at /rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library/core/src/panicking.rs:72:14
   2: core::panicking::assert_failed_inner
   3: core::panicking::assert_failed
   4: environmental::using
   5: core::ops::function::FnOnce::call_once
   6: core::ops::function::FnOnce::call_once
             at /rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library/core/src/ops/function.rs:250:5

acatangiu · 2024-04-15T08:07:40Z

Try merging master, unrelated CI errors should go away now

paritytech-cicd-pr · 2024-04-15T11:04:38Z

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: test-linux-stable 3/3
Logs: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/5928921

chungquantin · 2024-04-15T11:23:09Z

@acatangiu I think this PR is still impacted by the CI

ggwpez · 2024-04-15T12:22:05Z

Yes please fix the issue as pinged by the bot: #3867 (comment)

PS: Dont know why the error index is hard-coded, no wonder it fails. I would personally just do Err(crate::Error::<Test>::TooManyReserves.into()) instead of hard-coding error indices...

acatangiu · 2024-04-16T12:57:48Z

Yes please fix the issue as pinged by the bot: #3867 (comment)

PS: Dont know why the error index is hard-coded, no wonder it fails. I would personally just do Err(crate::Error::<Test>::TooManyReserves.into()) instead of hard-coding error indices...

Yes, good idea!

@chungquantin you can locally test this with cargo test -p pallet-xcm - fix the tests to pass locally, then should also pass in CI

chungquantin · 2024-04-18T03:24:24Z

@acatangiu @ggwpez I have updated the code to resolve issues with CI.

liamaharon · 2024-04-18T06:44:46Z

/tip small

substrate-tip-bot · 2024-04-18T06:45:20Z

@liamaharon A referendum for a small (20 DOT) tip was successfully submitted for @chungquantin (19nSqFQorfF2HxD3oBzWM3oCh4SaCRKWt1yvmgaPYGCo71J on polkadot).

Referendum number: 681.

substrate-tip-bot · 2024-04-18T06:45:56Z

The referendum has appeared on Polkassembly.

# Description - Link to issue: #3651 polkadot address: 19nSqFQorfF2HxD3oBzWM3oCh4SaCRKWt1yvmgaPYGCo71J

ggwpez · 2024-04-18T11:12:52Z

polkadot/xcm/pallet-xcm/src/lib.rs

 		UnableToDecode,
 		/// XCM encoded length is too large.
 		/// Returned when an XCM encoded length is larger than `MaxXcmEncodedSize`.
+		#[codec(index = 26)]


Not what i meant by:

I would personally just do Err(crate::Error::<Test>::TooManyReserves.into()) instead of hard-coding error indices

...

Please remove the hard-coded indices here:

polkadot-sdk/polkadot/xcm/pallet-xcm/src/tests/assets_transfer.rs

Line 826 in d0ebb85

error: [23, 0, 0, 0],

Oh I saw your suggestion. Should I revert and follow your aproach?

no revert required, you could do a followup PR

Gotcha thanks @acatangiu @ggwpez

Remove NotConcrete error

40abca0

chungquantin requested a review from a team as a code owner March 27, 2024 16:34

ggwpez reviewed Mar 27, 2024

View reviewed changes

polkadot/xcm/xcm-executor/src/traits/asset_transfer.rs Show resolved Hide resolved

Add prdoc and explicit coded index

37f4e30

acatangiu approved these changes Mar 28, 2024

View reviewed changes

chungquantin requested a review from ggwpez March 28, 2024 13:09

franciscoaguirre reviewed Mar 29, 2024

View reviewed changes

polkadot/xcm/xcm-executor/src/traits/asset_transfer.rs Show resolved Hide resolved

prdoc/pr_3867.prdoc Outdated Show resolved Hide resolved

polkadot/xcm/xcm-executor/src/traits/asset_transfer.rs Show resolved Hide resolved

Remove prdoc and update pallet-xcm

d1c65ce

- prdoc is not essential for a cleanup issue - Remove `InvalidAssetNonConcrete` type because this is unused if there is no `NotConcrete` type in asset transfer.

franciscoaguirre added the R0-silent Changes should not be mentioned in any release notes label Mar 29, 2024

franciscoaguirre reviewed Mar 29, 2024

View reviewed changes

polkadot/xcm/xcm-executor/src/traits/asset_transfer.rs Outdated Show resolved Hide resolved

Update asset_transfer.rs

819dae5

franciscoaguirre approved these changes Mar 29, 2024

View reviewed changes

ggwpez approved these changes Mar 29, 2024

View reviewed changes

Merge branch 'master' into chungquantin/remove_not_concrete

a1055c8

Merge branch 'master' into chungquantin/remove_not_concrete

c2e6c7b

Merge branch 'master' into chungquantin/remove_not_concrete

4872ce9

Add explicit codec indexes to the updated XCM error

6c42f76

liamaharon approved these changes Apr 18, 2024

View reviewed changes

liamaharon added this pull request to the merge queue Apr 18, 2024

Merged via the queue into paritytech:master with commit d591b16 Apr 18, 2024
132 of 136 checks passed

chungquantin deleted the chungquantin/remove_not_concrete branch April 18, 2024 07:12

sandreim pushed a commit that referenced this pull request Apr 18, 2024

Remove NotConcrete error (#3867)

e29535f

# Description - Link to issue: #3651 polkadot address: 19nSqFQorfF2HxD3oBzWM3oCh4SaCRKWt1yvmgaPYGCo71J

ggwpez reviewed Apr 18, 2024

View reviewed changes

chungquantin restored the chungquantin/remove_not_concrete branch April 18, 2024 11:14

chungquantin deleted the chungquantin/remove_not_concrete branch April 18, 2024 11:18

ggwpez mentioned this pull request Apr 22, 2024

[tests] Remove hard-coded error indices in pallet-xcm tests #4237

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove NotConcrete error #3867

Remove NotConcrete error #3867

chungquantin commented Mar 27, 2024 •

edited

Loading

franciscoaguirre left a comment

chungquantin commented Mar 30, 2024

acatangiu commented Apr 15, 2024 •

edited

Loading

paritytech-cicd-pr commented Apr 15, 2024

chungquantin commented Apr 15, 2024

ggwpez commented Apr 15, 2024 •

edited

Loading

acatangiu commented Apr 16, 2024

chungquantin commented Apr 18, 2024

liamaharon commented Apr 18, 2024

substrate-tip-bot bot commented Apr 18, 2024

substrate-tip-bot bot commented Apr 18, 2024

ggwpez Apr 18, 2024 •

edited

Loading

chungquantin Apr 18, 2024

acatangiu Apr 18, 2024

chungquantin Apr 18, 2024

Remove NotConcrete error #3867

Remove NotConcrete error #3867

Conversation

chungquantin commented Mar 27, 2024 • edited Loading

Description

franciscoaguirre left a comment

Choose a reason for hiding this comment

chungquantin commented Mar 30, 2024

acatangiu commented Apr 15, 2024 • edited Loading

paritytech-cicd-pr commented Apr 15, 2024

chungquantin commented Apr 15, 2024

ggwpez commented Apr 15, 2024 • edited Loading

acatangiu commented Apr 16, 2024

chungquantin commented Apr 18, 2024

liamaharon commented Apr 18, 2024

substrate-tip-bot bot commented Apr 18, 2024

substrate-tip-bot bot commented Apr 18, 2024

ggwpez Apr 18, 2024 • edited Loading

Choose a reason for hiding this comment

chungquantin Apr 18, 2024

Choose a reason for hiding this comment

acatangiu Apr 18, 2024

Choose a reason for hiding this comment

chungquantin Apr 18, 2024

Choose a reason for hiding this comment

chungquantin commented Mar 27, 2024 •

edited

Loading

acatangiu commented Apr 15, 2024 •

edited

Loading

ggwpez commented Apr 15, 2024 •

edited

Loading

ggwpez Apr 18, 2024 •

edited

Loading