Skip to content

Update provider registry and application registry via governance #2587

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
saraswatpuneet merged 29 commits into from
Sep 6, 2025
Merged

Update provider registry and application registry via governance #2587

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
566faae
add update_provider_via_governance extrinsic
saraswatpuneet Sep 2, 2025
c6a6446
provider update related ext
saraswatpuneet Sep 2, 2025
ff260b4
add extrinsics to update a given application
saraswatpuneet Sep 2, 2025
fa1dfcb
add provider/application update tests
saraswatpuneet Sep 3, 2025
e8d61b7
add benchmarks and tests
saraswatpuneet Sep 3, 2025
ce09338
Update weights
saraswatpuneet Sep 3, 2025
02208fe
update weights and add e2e tests
saraswatpuneet Sep 3, 2025
5f47790
lint e2e
saraswatpuneet Sep 3, 2025
3e53ac9
Merge branch 'feat/provider-context-development' into 2549_update_pro…
saraswatpuneet Sep 3, 2025
8fcd4d3
add a non sudo required create application function and block it in r…
saraswatpuneet Sep 4, 2025
fa789ef
cleanups
saraswatpuneet Sep 4, 2025
19dac9b
better assertions
saraswatpuneet Sep 4, 2025
232bf98
update benchmarks to run over a lenght of locales
saraswatpuneet Sep 4, 2025
013b982
Update weights
saraswatpuneet Sep 4, 2025
156ae37
make benchmakrs more grandular
saraswatpuneet Sep 4, 2025
9af87fb
temporary: to make benchmarks run with m,n
saraswatpuneet Sep 4, 2025
720a627
Update weights
saraswatpuneet Sep 4, 2025
a8e49d7
improve benchmarks
saraswatpuneet Sep 4, 2025
e7f01ed
Update weights
saraswatpuneet Sep 4, 2025
d7451c3
set correct weights on extrinsics
saraswatpuneet Sep 4, 2025
1dab27b
Update weights
saraswatpuneet Sep 4, 2025
1fa7330
fix lint
saraswatpuneet Sep 4, 2025
7e40764
block on filter
saraswatpuneet Sep 4, 2025
5680042
remove hidden in test
saraswatpuneet Sep 5, 2025
c6a4a74
update pallet readme with new apis and runtime api
saraswatpuneet Sep 5, 2025
6517305
set proper removabls of older cids from logo storage
saraswatpuneet Sep 5, 2025
a8643af
Update weights
saraswatpuneet Sep 5, 2025
b944fe8
better assertions
saraswatpuneet Sep 6, 2025
267f015
remove unused variable
saraswatpuneet Sep 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
add a non sudo required create application function and block it in r… 
…untime
  • Loading branch information
@saraswatpuneet
saraswatpuneet committed Sep 4, 2025
commit 8fcd4d321e6efe1c26dad6058863ef9361464649
33 changes: 31 additions & 2 deletions pallets/msa/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,7 @@ pub mod pallet {
/// Application for provider created
ApplicationCreated {
/// The MSA id associated with the provider
msa_id: ProviderId,
provider_id: ProviderId,
/// The application id for the created application
application_id: ApplicationIndex,
},
Expand Down Expand Up @@ -1476,7 +1476,7 @@ pub mod pallet {
let application_id =
Self::create_application_for(ProviderId(provider_msa_id), payload)?;
Self::deposit_event(Event::ApplicationCreated {
msa_id: ProviderId(provider_msa_id),
provider_id: ProviderId(provider_msa_id),
application_id,
});
Ok(())
Expand Down Expand Up @@ -1680,6 +1680,35 @@ pub mod pallet {
T::ProposalProvider::propose(proposer, threshold, proposal)?;
Ok(())
}

/// Create application allows creating an application registry without governance
/// This call is blocked in release mode
#[pallet::call_index(29)]
#[pallet::weight(T::WeightInfo::create_application_via_governance())]
pub fn create_application(
origin: OriginFor<T>,
payload: ApplicationContext<
T::MaxProviderNameSize,
T::MaxLanguageCodeSize,
T::MaxLogoCidSize,
T::MaxLocaleCount,
>,
) -> DispatchResult {
let provider_account = ensure_signed(origin)?;
let provider_msa_id = Self::ensure_valid_msa_key(&provider_account)?;
ensure!(
Self::is_registered_provider(provider_msa_id),
Error::<T>::ProviderNotRegistered
);
Self::ensure_correct_cids(&payload)?;
let application_id =
Self::create_application_for(ProviderId(provider_msa_id), payload)?;
Self::deposit_event(Event::ApplicationCreated {
provider_id: ProviderId(provider_msa_id),
application_id,
});
Ok(())
}
}
}

Expand Down
37 changes: 37 additions & 0 deletions pallets/msa/src/tests/application_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,43 @@ fn upload_logo_fails_mismatch_logo_data() {
});
}

#[test]
fn create_application_and_upload_logo() {
new_test_ext().execute_with(|| {
let (_, key_pair) = create_provider_with_name("LogoProvider");
let logo_data = include_bytes!("../../../../e2e/msa/frequency.png");
let cid = common_primitives::cid::compute_cid_v1(logo_data).expect("Failed to compute CID");
let encoded = multibase::encode(multibase::Base::Base58Btc, cid);
let input_bounded_cid = BoundedVec::try_from(encoded.as_bytes().to_vec()).unwrap();
let bounded_logo_data =
BoundedVec::<u8, common_runtime::constants::MsaMaxLogoSize>::try_from(
logo_data.to_vec(),
)
.expect("Failed to create BoundedVec");
let entry = ApplicationContext {
default_logo_250_100_png_cid: input_bounded_cid.clone(),
..Default::default()
};
assert_ok!(Msa::create_application(RuntimeOrigin::signed(key_pair.into()), entry));

assert!(ApprovedLogos::<Test>::contains_key(&input_bounded_cid));
let stored_logo_data =
ApprovedLogos::<Test>::get(&input_bounded_cid).expect("Logo should be approved");
assert!(stored_logo_data.is_empty());

assert_ok!(Msa::upload_logo(
RuntimeOrigin::signed(key_pair.into()),
input_bounded_cid.clone(),
bounded_logo_data.clone()
));

assert!(ApprovedLogos::<Test>::contains_key(&input_bounded_cid));
let stored_bytes =
ApprovedLogos::<Test>::get(&input_bounded_cid).expect("Logo should be approved");
assert_eq!(stored_bytes, bounded_logo_data);
});
}

#[test]
fn compute_cid_v1_test() {
new_test_ext().execute_with(|| {
Expand Down
8 changes: 5 additions & 3 deletions runtime/frequency/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,8 +311,10 @@ impl BaseCallFilter {
// Block all `FrequencyTxPayment` calls from utility batch
RuntimeCall::FrequencyTxPayment(..) => false,

// Block `create_provider_v2` and `create_schema` calls from utility batch
// Block `create_provider`, `create_provider_v2`, `create_application` and `create_schema` calls from utility batch
RuntimeCall::Msa(pallet_msa::Call::create_provider { .. }) |
RuntimeCall::Msa(pallet_msa::Call::create_provider_v2 { .. }) |
RuntimeCall::Msa(pallet_msa::Call::create_application { .. }) |
Comment on lines +315 to +318
Copy link
Collaborator

@JoeCap08055 JoeCap08055 Sep 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, for a while I've wondered why we filter the calls this way, instead of simply feature-flagging those extrinsics out for a mainnet build.

Doing the call filtering this way, the extrinsics still show up in the metadata on mainnet, but you can't call them... but you don't know that until you try to call them...

Copy link
Collaborator Author

@saraswatpuneet saraswatpuneet Sep 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this only block capacity tx, one can use sudo/ council keys with tokens to by pass this ? which is okay i guess

Copy link
Collaborator

@JoeCap08055 JoeCap08055 Sep 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I meant to point out line 263 above, where we filter out for mainnet

Copy link
Collaborator Author

@saraswatpuneet saraswatpuneet Sep 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see what you mean, good catch I will add it there too, this runtime is getting huge

Copy link
Collaborator

@JoeCap08055 JoeCap08055 Sep 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right--so my question is, why do we filter the calls from mainnet instead of just feature-flagging the extrinsics out?

Copy link
Collaborator

@JoeCap08055 JoeCap08055 Sep 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to raise the question as a parking lot next week in standup; maybe we'll create an issue based on the outcome of the discussion.

FYI here's what AI said (with my comments appended)

Comparison of BaseCallFilter vs. feature flags

Aspect BaseCallFilter (Runtime Filtering) Feature Flags (Compile-time Removal) Comments
Security Less Secure. The extrinsic's code is present in the production binary. A bug in your filter logic could allow a malicious user to call the extrinsic on the main network. This increases the chain's attack surface. Most Secure. The extrinsic's code is completely removed from the production binary. It is impossible for an attacker to call an extrinsic that does not exist in the Wasm.
Code Size Larger. The production binary includes the code for both the governance-only and the dev/test extrinsics, contributing to a larger Wasm size. Smaller. The production binary contains only the code necessary for the main network. This can reduce on-chain storage costs and improve performance.
Maintainability Simpler. The core business logic is in one place. Developers don't need to manage conditional compilation or different build configurations. More complex. Requires careful management of build configurations for different environments (dev, testnet, production). You must ensure the correct flags are set during the build process. We already manage multi-environment feature flags, so this would not be an extra burden
Environment Configuration More Flexible. You can manage the logic at runtime by changing a configuration storage item (e.g., through a sudo call in dev, or a governance call in production). This can be useful for dynamic changes or temporary overrides. Less Flexible. Configuration is static and baked into the Wasm binary at compile time. Any change requires a full runtime upgrade. I don't think runtime flexibility is needed/desired for Frequency here
Benchmarking Less Accurate. Benchmarks must account for the code path of the dev-only extrinsic, even though it will not run in production. This can lead to a less efficient and larger-than-needed weight calculation. More Accurate. Benchmarks only measure the production code paths, resulting in more precise and optimized weight calculations. No impact, we don't have mixed code paths. The dev-only extrinsics are completely separate from the governance ones.
Clarity of Intent Potentially Confusing. Developers or chain explorers may see the create_something extrinsic available in the production runtime, even though it is filtered. This can be misleading. Highly Clear. The code base explicitly shows which extrinsics are for development purposes only. polkadot.js and other UIs will not show the dev-only extrinsic for the production runtime.

Copy link
Collaborator Author

@saraswatpuneet saraswatpuneet Sep 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's pretty useful thank you and I agree this is worth discussing we can start to optimize the runtime.

Also linked above comment to main Provider Context PR

Copy link
Collaborator

@aramikm aramikm Sep 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the problem with having features on top of extrinsics was that it would modify the extrinsic indices between testnet and mainnet when we didn't have explicit index assignment for each extrinsic.

The other issue is the metadata compatibility. Having metadata for a feature that is disabled is better compared to not having any metadata for it. If we gate extrinisics by feature it would fork metadata into 2 different versions depending on if it was generated for testnet or mainnnet which can also be another pain point in future.

I think if we decide to add feature to these it should probably be inside the extrinsic body and the implementation should be refactored to support this.

Copy link
Collaborator

@JoeCap08055 JoeCap08055 Sep 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm... regarding the extrinsic index, isn't this set explicitly by #[pallet::call_index(5)], for instance? We already have "holes" in the extrinsic indices for deprecated extrinsics that have been completely removed.

Also, the metadata is already different between chains, since we remove the entire sudo pallet for Mainnet.

Copy link
Collaborator

@aramikm aramikm Sep 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes as mentioned in my previous comment initially we didn't have that explicit indexing capability but since it's that is fixed the only argument about having these as a filter would be to keep the metadata as consistent as possible.

The argument about sudo pallet is correct but that seems to be an exceptional case I'm not sure how metadata difference would affect when filtering an extrinsic by a feature. It might be alright or it might cause issues. The problem is that if we generate the metadata with mainnet then we can never call those extrinsics from e2e tests which is not great and if we decide to generate the metadata for paseo then we would have metadata for a featurer which does not exists on the chain and that might cause other issues.

RuntimeCall::Schemas(pallet_schemas::Call::create_schema_v3 { .. }) => false,

// Block `Pays::No` calls from utility batch
Expand Down Expand Up @@ -645,7 +647,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
spec_name: Cow::Borrowed("frequency"),
impl_name: Cow::Borrowed("frequency"),
authoring_version: 1,
spec_version: 176,
spec_version: 177,
impl_version: 0,
apis: RUNTIME_API_VERSIONS,
transaction_version: 1,
Expand All @@ -659,7 +661,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
spec_name: Cow::Borrowed("frequency-testnet"),
impl_name: Cow::Borrowed("frequency"),
authoring_version: 1,
spec_version: 176,
spec_version: 177,
impl_version: 0,
apis: RUNTIME_API_VERSIONS,
transaction_version: 1,
Expand Down
Loading