rmrk-team / rmrk-substrate

[WIP] RMRK Substrate pallets
https://rmrk.app
Other
74 stars 36 forks source link

Disable pallet uniques calls #154

Closed ilionic closed 2 years ago

ilionic commented 2 years ago

Fixes #151 Fixes #150

bmacer commented 2 years ago

Looks good. I tried to implement a test of this on rmrk-core but failed. Not sure how I did it wrong.

I added this to the mock.rs https://github.com/rmrk-team/rmrk-substrate/blob/036cc638fadd55df2081503e2bbd0ff69f1bca05/runtime/src/lib.rs#L169-L193

Changed BaseCallFilter to equal BaseFilter https://github.com/rmrk-team/rmrk-substrate/blob/036cc638fadd55df2081503e2bbd0ff69f1bca05/runtime/src/lib.rs#L199

And wrote this test (not really a test yet just a println, but the result is unexpected/unwanted "Ok(())"

/// Uniques: Unable to call directly
#[test]
fn calling_uniques_directly_fails() {
    ExtBuilder::default().build().execute_with(|| {
        println!("{:?}", Uniques::create(Origin::signed(ALICE),1, ALICE));
    });
}

If a test for this doesn't matter that much, go ahead and merge. Otherwise, any ideas?

HashWarlock commented 2 years ago

Looks good. I tried to implement a test of this on rmrk-core but failed. Not sure how I did it wrong.

I added this to the mock.rs

https://github.com/rmrk-team/rmrk-substrate/blob/036cc638fadd55df2081503e2bbd0ff69f1bca05/runtime/src/lib.rs#L169-L193

Changed BaseCallFilter to equal BaseFilter

https://github.com/rmrk-team/rmrk-substrate/blob/036cc638fadd55df2081503e2bbd0ff69f1bca05/runtime/src/lib.rs#L199

And wrote this test (not really a test yet just a println, but the result is unexpected/unwanted "Ok(())"

/// Uniques: Unable to call directly
#[test]
fn calling_uniques_directly_fails() {
  ExtBuilder::default().build().execute_with(|| {
      println!("{:?}", Uniques::create(Origin::signed(ALICE),1, ALICE));
  });
}

If a test for this doesn't matter that much, go ahead and merge. Otherwise, any ideas?

Check out the test file in substrate and this example https://github.com/paritytech/substrate/blob/master/frame/utility/src/tests.rs#L277-L292

We should get an frame_system::Error::<Test>::CallFiltered.

ilionic commented 2 years ago

Will merge for now as filtered Calls not part of rmrk pallets technically. But I guess good to add a test anyway