Skip to content
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.

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

refactor(mainnet): pallet_proxy config & tests #441

Merged

Conversation

al3mart
Copy link
Collaborator

@al3mart al3mart commented Jan 21, 2025

Extends proxy config module for the pallet:

  • pallet_proxy

Notable configuration items are:

  • pallet_proxy

Updated deposits for mainnet runtime:

// One storage item; key size 32 + 8.
pub const ProxyDepositBase: Balance = deposit(1, 40);
// Additional storage item size of AccountId 32 bytes + ProxyType 1 byte + BlockNum 4 bytes.
pub const ProxyDepositFactor: Balance = deposit(0, 37);
// One storage item; key size 32, value size 16.
pub const AnnouncementDepositBase: Balance = deposit(1, 48);
// Additional storage item 32 bytes AccountId + 32 bytes Hash + 4 bytes BlockNum.
pub const AnnouncementDepositFactor: Balance = deposit(0, 68);
type MaxPending = 32;
type MaxProxies = 32;

[sc-2205]

@al3mart al3mart force-pushed the al3mart/refactor-mainnet-config branch 3 times, most recently from 7359e7a to 9594a3b Compare January 24, 2025 17:20
@al3mart al3mart force-pushed the al3mart/refactor-mainnet-proxy branch from c4126d4 to f73d4e5 Compare January 27, 2025 17:25
@al3mart al3mart marked this pull request as ready for review January 27, 2025 18:06
@codecov-commenter
Copy link

codecov-commenter commented Jan 27, 2025

Codecov Report

Attention: Patch coverage is 93.68421% with 6 lines in your changes missing coverage. Please review.

Project coverage is 71.46%. Comparing base (9594a3b) to head (f318840).
Report is 2 commits behind head on al3mart/refactor-mainnet-config.

Files with missing lines Patch % Lines
runtime/mainnet/src/config/proxy.rs 93.68% 6 Missing ⚠️
@@                         Coverage Diff                         @@
##           al3mart/refactor-mainnet-config     #441      +/-   ##
===================================================================
+ Coverage                            71.02%   71.46%   +0.44%     
===================================================================
  Files                                   73       75       +2     
  Lines                                13748    13971     +223     
  Branches                             13748    13971     +223     
===================================================================
+ Hits                                  9764     9984     +220     
- Misses                                3717     3720       +3     
  Partials                               267      267              
Files with missing lines Coverage Δ
runtime/mainnet/src/config/proxy.rs 83.17% <93.68%> (+83.17%) ⬆️

... and 3 files with indirect coverage changes

Copy link
Collaborator

@peterwht peterwht left a comment

Choose a reason for hiding this comment

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

Only concern is around the ProxyDepositBase and ProxyDepositFactor.

  1. Wasn't able to calculate the value size myself (so not sure where the 16 comes from)
  2. Our deposits are several times more expensive than the polkadot relay.

@@ -41,6 +39,17 @@ impl InstanceFilter<RuntimeCall> for ProxyType {
}
}

parameter_types! {
// One storage item; key size 32, value size 16.
pub const ProxyDepositBase: Balance = deposit(1, 48);
Copy link
Collaborator

Choose a reason for hiding this comment

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

we have 6 times the amount of bytes Polkadot has. Any reason not to just align to the Polkadot relay chain on this: https://github.com/polkadot-fellows/runtimes/blob/main/relay/polkadot/src/lib.rs#L977

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Certainly. Double checking after your comment I'm also seeing that I did the following miscalculation:

the number we have for ProxyDepositBase was the result of adding AccountId + Balance, but I realize that we should be accounting for 8 bytes, which is the correct length of the key of the map.

I also added the bytes for Balance because the deposited amount is always stored. Although, as it is the size corresponding to the deposit itself, I could see why it is not being included on Polkadot's configuration.

ProxyDepositFactor reasoning in the comment above its declaration, what's the concern ?

Copy link
Collaborator Author

@al3mart al3mart Jan 29, 2025

Choose a reason for hiding this comment

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

d4c936e

As discussed off GH this change updates ProxyDepositBase from deposit(1, 48) to deposit(1, 40).

@al3mart al3mart force-pushed the al3mart/refactor-mainnet-proxy branch from 0716b86 to cd7ebb2 Compare January 29, 2025 18:48
Copy link
Collaborator

@peterwht peterwht left a comment

Choose a reason for hiding this comment

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

Nice! Thanks for resolving the deposit issue. Makes much more sense now.

Copy link
Collaborator

@Daanvdplas Daanvdplas left a comment

Choose a reason for hiding this comment

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

Missing the explanation why are we not using the values from pop-common anymore. Other than that, looking good!

@al3mart al3mart requested a review from Daanvdplas February 3, 2025 13:11
Copy link
Collaborator

@Daanvdplas Daanvdplas left a comment

Choose a reason for hiding this comment

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

Nice! Thanks for the updated tests, very good!

@al3mart al3mart merged commit b71c3a8 into al3mart/refactor-mainnet-config Feb 4, 2025
15 checks passed
@al3mart al3mart deleted the al3mart/refactor-mainnet-proxy branch February 4, 2025 09:37
al3mart added a commit that referenced this pull request Feb 7, 2025
* refactor(mainnet): pallet_proxy config & tests

* style(proxy): tests follow config order

* refactor(proxy): remove asset related ProxyTypes

* refactor(proxy): update ProxyDepositBase price

* refactor(proxy): revert d3bfc6c and redefine ProxyType for mainnet

typo

* fix(proxy): amend proxy_has_deposit_base test

* docs(proxy): Clarify new ProxyType definition

* docs(proxy): add is_superset comments

* test(proxy): clarify deposit byte size lenght
al3mart added a commit that referenced this pull request Feb 17, 2025
* refactor(mainnet): pallet_proxy config & tests

* style(proxy): tests follow config order

* refactor(proxy): remove asset related ProxyTypes

* refactor(proxy): update ProxyDepositBase price

* refactor(proxy): revert d3bfc6c and redefine ProxyType for mainnet

typo

* fix(proxy): amend proxy_has_deposit_base test

* docs(proxy): Clarify new ProxyType definition

* docs(proxy): add is_superset comments

* test(proxy): clarify deposit byte size lenght
al3mart added a commit that referenced this pull request Feb 17, 2025
* refactor(mainnet): add governance & pallet_sudo tests (#443)

* refactor(mainnet): add governance & pallet_sudo tests

* refactor(sudo): sudo account defined in governance & unit test

fix(governance): make governance mod public & better sudo declaration

* refactor(mainnet): utility config (#438)

* refactor(mainnet): add pallet_multisig to utility

* test(mainnet): add pallet_multisig tests

* refactor(mainnet): add pallet_utility to utility

* test(mainnet): add pallet_utility tests

* refactor(mainnet): add pallet_preimage to utility

* test(mainnet): add pallet_preimage tests

* refactor(mainnet): add pallet_scheduler to utility

* test(mainnet): add pallet_scheduler tests

* refactor(mainnet): pallet_proxy config & tests (#441)

* refactor(mainnet): pallet_proxy config & tests

* style(proxy): tests follow config order

* refactor(proxy): remove asset related ProxyTypes

* refactor(proxy): update ProxyDepositBase price

* refactor(proxy): revert d3bfc6c and redefine ProxyType for mainnet

typo

* fix(proxy): amend proxy_has_deposit_base test

* docs(proxy): Clarify new ProxyType definition

* docs(proxy): add is_superset comments

* test(proxy): clarify deposit byte size lenght

* refactor(mainnet): system config (#442)

* refactor(mainnet): add frame_system to system

* test(mainnet): add pallet_system test

* fix(mainnet): rebase fixes

* fix(system): bring missing types in scope

* refactor(system): add cumulus_parachain_system to system config module

* test(system): add cumulus_parachain_system tests

* refactor(system): add parachain_info into system config module

* refactor(system): add pallet_timestamp into system config module

* style(system): add ConsensusHook comment

* refactor(system): separate tests by module

* docs(system): document public config modules

* fix(system): allow unused_imports to remove Executive warning

* refactor(system): reintroduce FilteredCalls

* style(system): get rid of test prefixes

* style(system): relocate max_block_weigth comment

* test(system): check for Hash type config

* docs(system): amend outdated comments

* Update runtime/mainnet/src/config/mod.rs

Co-authored-by: Daan van der Plas <[email protected]>

* Update runtime/mainnet/src/config/mod.rs

Co-authored-by: Daan van der Plas <[email protected]>

* Update runtime/mainnet/src/config/mod.rs

Co-authored-by: Daan van der Plas <[email protected]>

* Update runtime/mainnet/src/config/mod.rs

Co-authored-by: Daan van der Plas <[email protected]>

---------

Co-authored-by: Daan van der Plas <[email protected]>

* refactor(mainnet): monetary config (#440)

* refactor(mainnet): pallet_balances & fee constants

* test(mainnet): add pallet_balances tests

* refactor(mainnet): add pallet_transaction_payment to monetary config

* test(mainnet): add transaction_payment tests

* fix(mainnet): rebase fixes

* fix(monetary): don't burn dust

* refactor(monetary): use treasury account instead of sudo

remove extra clone

* refactor(monetary): use existential_deposit instead of UNIT in dust test

* refactor(monetary): Don't use DOLLARS & CENTS is defined in terms of UNITS

* fix(monetary): missing doc for fn deposit

* docs(monetary): remove src from docs

* docs(monetary): update TransactionByteFee comments

* refactor(monetary): reorg tests into modules

* style(monetary): remove confussing comment

* style(monetar): remove prefixes from tests

changes after rebase

more changes after reabse

* refactor(mainnet): collation config (#439)

* refactor(mainnet): add pallet_authorship to collation

* test(mainnet): add pallet_auhtorship test

* refactor(mainnet): add pallet_aura to collation

* test(mainnet): add pallet_aura tests

* refactor(mainnet): add collator_selection to collation

* test(mainnet): add collator_selection tests

* refactor(mainnet): add pallet_session & aura_ext to collation

* test(mainnet): add pallet_session tests

* chore(aura): MaxAuthorities matches max possible blocks produced

* remove unnused imports

* style(collation): order tests following pallet config

* style(collation): remove unnecessary qualifications

* style(collation): max_authorities_is_3600

* style(collation): reorg definition in session_keys_provided_by_aura test

* style(collation): better Period  comment

* style(collation): clarify session::SessionHandler comment

* refactor(collation): separate tests in modules by pallet

* style(collation): Explain SessionHandler config

fixes after rebase

* style(collation): remove prefixes from tests

* style(collation): Period comment was not helpful

changes after rebase

* style(utility): better test structure

style(utility): fmt

rebase

fmt

fmt

* refactor(mainnet): xcm config (#455)

* refactor(mainnet): rebase changes

* chore(mainnet): update Cargo.lock

* refactor(mainnet): utility config (#438)

* refactor(mainnet): add pallet_multisig to utility

* test(mainnet): add pallet_multisig tests

* refactor(mainnet): add pallet_utility to utility

* test(mainnet): add pallet_utility tests

* refactor(mainnet): add pallet_preimage to utility

* test(mainnet): add pallet_preimage tests

* refactor(mainnet): add pallet_scheduler to utility

* test(mainnet): add pallet_scheduler tests

* refactor(mainnet): system config (#442)

* refactor(mainnet): add frame_system to system

* test(mainnet): add pallet_system test

* fix(mainnet): rebase fixes

* fix(system): bring missing types in scope

* refactor(system): add cumulus_parachain_system to system config module

* test(system): add cumulus_parachain_system tests

* refactor(system): add parachain_info into system config module

* refactor(system): add pallet_timestamp into system config module

* style(system): add ConsensusHook comment

* refactor(system): separate tests by module

* docs(system): document public config modules

* fix(system): allow unused_imports to remove Executive warning

* refactor(system): reintroduce FilteredCalls

* style(system): get rid of test prefixes

* style(system): relocate max_block_weigth comment

* test(system): check for Hash type config

* docs(system): amend outdated comments

* Update runtime/mainnet/src/config/mod.rs

Co-authored-by: Daan van der Plas <[email protected]>

* Update runtime/mainnet/src/config/mod.rs

Co-authored-by: Daan van der Plas <[email protected]>

* Update runtime/mainnet/src/config/mod.rs

Co-authored-by: Daan van der Plas <[email protected]>

* Update runtime/mainnet/src/config/mod.rs

Co-authored-by: Daan van der Plas <[email protected]>

---------

Co-authored-by: Daan van der Plas <[email protected]>

* refactor(mainnet): collation config (#439)

* refactor(mainnet): add pallet_authorship to collation

* test(mainnet): add pallet_auhtorship test

* refactor(mainnet): add pallet_aura to collation

* test(mainnet): add pallet_aura tests

* refactor(mainnet): add collator_selection to collation

* test(mainnet): add collator_selection tests

* refactor(mainnet): add pallet_session & aura_ext to collation

* test(mainnet): add pallet_session tests

* chore(aura): MaxAuthorities matches max possible blocks produced

* remove unnused imports

* style(collation): order tests following pallet config

* style(collation): remove unnecessary qualifications

* style(collation): max_authorities_is_3600

* style(collation): reorg definition in session_keys_provided_by_aura test

* style(collation): better Period  comment

* style(collation): clarify session::SessionHandler comment

* refactor(collation): separate tests in modules by pallet

* style(collation): Explain SessionHandler config

fixes after rebase

* style(collation): remove prefixes from tests

* style(collation): Period comment was not helpful

changes after rebase

* refactor(xcm): add pallet_message_queue into xcm config module

* test(xcm): add message_queue tests

* test(xcm): test location, transactor and barrier type definitions

* refactor(xcm): include pallet_xcmp_queue into xcm config module

* test(xcm): add pallet_xcmp_queue configuration tests

* test(xcm): add xcm_executor configuration tests

* test(xcm): adding pallet_xcm config tests

* fix(xcm): typo

fix(xcm): XcmReserveTransferFilter is Everything

* refactor(xcm): use unitype for MaxRemoteLockConsumers

* style(xcm): tests separated in modules

* style(xcm): better naming for XcmTeleportFilter test

* style(xcm): test naming and order

* docs(xcm): clarifying comments for specific xcmp_queueu types

* style(xcm): typo

* style(xcm): better name for reserves tests

* refactor(xcm): Use `NativeAssetFrom<T>` as `XcmReserveTransferFilter` (#459)

* refactor(xcm): Use NativeAssetFrom<T> as XcmReserveTransferFilter

style:fmt

* fix(xcm): NativeAssetFrom<AssetHub> filters everything but DOT from AH

* fix(xcm): XcmReserveTransferFilter filters only by asset

fix

changes after rebase

fmt

* fix(xcm): remove waived locations (#456)

* fix(xcm): no locations are waived

* test(xcm): improved waived_location test

changes after rebase

* style(xcm): clarify that we are not waiving fees for any location

* fix(xcm): charge delivery fees (#457)

* resolve rebase conflicts

* fix(xcm): reserve_transfer_native_asset_from_para_to_system_para accounts for delivery fees

* fix(integration-tests): handle delivery_fees only with mainnet feature on

fmt after rebase

fmt

changes after rebase

fmt

fmt

* test(xcm): address feedback from #459

typos

AssetFilter

* style(xcm): locate reserve tests within executor

lint

* fix(integration-test): improve tests & remove feature gated compilation

unnecessary Fungibles

* refactor: integration tests mainnet xcm (#466)

* refactor: integration tests mainnet xcm

* fix: remove unused ED

rebase

fmt

fmt

* Update runtime/mainnet/src/config/xcm.rs

Co-authored-by: Daan van der Plas <[email protected]>

* refactor(xcm): use TreasuryAccount from monetary

* chore(xcm): unused imports

* chore(xcm): amend test modules

* chore(integration-tests): amend reserve_transfer_native_asset_from_para_to_system

* chore(xcm): divive reserve filter test in two

* chore(xcm): fmt

* style(integration-tests): fix unwanted indentation

* test(xcm): more focused unit tests

* Update runtime/mainnet/src/config/xcm.rs

Co-authored-by: Daan van der Plas <[email protected]>

* Update runtime/mainnet/src/config/xcm.rs

Co-authored-by: Daan van der Plas <[email protected]>

* fix(test): amend suggestions

---------

Co-authored-by: Daan van der Plas <[email protected]>

* feat(mainnet): introduce assets (#465)

* refactor(mainnet): include assets

fmt

refactor(collator): not necessary import

* test(assets): add nfts config unit tests

* refactor(proxy): Add assets & smart contract proxy type

* refactor(assets): implement nfts runtime api

* fix(pallet-nfts-api): workspace linters are not defined

* docs(assets): clarify deposits

* chore(proxy): improve usage of references

* docs(assets): update src reference

* chore(assets): update MetadataDepositBase

* docs(assets): remove TODOs

* refactor(proxy): revert SmartContract ProxyType

* revert: e614927

* style(Cargo): revert format changes on dep comments

* chore(assets): apply feedback to tests

* chore(assets): removed unused type

* revert: fb92342

This reverts commit fb92342.

* docs(assets): better comments

* chore(assets): lower nfts deposit costs

* chore(assets): reduced deposit cost for AssetAccountDeposit

* chore(assets): use SDK's pallet_nfts

* chore(assets): better deposits & fmt Cargo

* chore(assets): better comments

* chore(assets): more explicit comments for deposits

* chore(assets): provide src for NftsCollectionDeposit deposit

* chore(assets): apply feedback

* chore(monetary): clean unnecessary comments

* chore(utility): make deposit calc explicit. (#469)

chore(utility): explicit deposit justification

rebase fixes

* chore(proxy): reintroduce Asset related proxy types

* syle(proxy): Clarify proxy test comments

---------

Co-authored-by: Daan van der Plas <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants