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

Access Controller Fee Vault. #1785

Merged
merged 19 commits into from
Apr 29, 2024
Merged

Access Controller Fee Vault. #1785

merged 19 commits into from
Apr 29, 2024

Conversation

0xOmarA
Copy link
Member

@0xOmarA 0xOmarA commented Apr 23, 2024

Summary

This PR adds an XRD vault to the state of access controllers and methods for depositing, withdrawing, and locking XRD from that vault for fees. Anyone is allowed to deposit into the vault. Only the primary role is allowed to withdraw from the vault and any three of the roles are allowed to lock XRD from the vault for fees.

Details

An AccessControllerV2 blueprint is added with this PR which has an updates state structure that contains an XRD whose primary usage is for locking fees for recoveries. The addition of this field pushed the state to v2. The update from the v1 state (that doesn't have the vault) to v2 (which has a field for an optional vault) is done lazily, any method call to the access controller updates the state after the protocol update. This means that it is possible to have some access controller on v1 while others are on v2 after the protocol update.

Alongside the above state changes, the following methods have been added to the access controller blueprint:

// Requires P, C, or R
pub fn lock_recovery_fee(&mut self, amount: Decimal);

// Requires P
pub fn withdraw_recovery_fee(&mut self, amount: Decimal) -> Bucket;

// Anyone can call
pub fn contribute_recovery_fee(&mut self, bucket: Bucket);

A DepositRecoveryXrdEvent is emitted when XRD is deposited into the vault via the contribute_recovery_fee method and a WithdrawRecoveryXrdEvent method is emitted when XRD is withdrawn from the vault via the withdraw_recovery_fee method.

Testing

All of the testing done for this PR can be found in the radix-engine-tests/tests/flash/access_controller.rs file.

Update Recommendations

For Internal Integrators

Be aware that this introduces a protocol update that updates the access controller blueprint from v1 to v2 by doing the following:

  • Removes the old schema substate of the package and replacing it with a new one in a different key (different schema hash)
  • Removes the old original code substate of the package and replaces it with a new one in a different key (different original code hash)
  • Removes the old vm type substate of the package and replaces it with a new one in a different key (different original code hash)
  • Updates the blueprint definition with the newly added methods, change in the code hash, change in the schema hash, and added events.
  • Updates the blueprint auth configs with the newly added methods.

Additionally, the following changes were made that you should be aware of:

  • The schema of the access controller state has changed.
  • It is possible to encounter access controllers with v1 state and others with v2 state after the protocol update.
  • The code id of the access controller package has changed.
  • The access controller blueprint now has three more methods.
  • The set of "native" events has increased by 2.

@0xOmarA 0xOmarA added scrypto-lib Any change to the scrypto library native-blueprint-interface Any change to the interface of native blueprints. NOTE: Also tag with `manifest` and `scrypto-lib` files-moved labels Apr 23, 2024
Copy link

github-actions bot commented Apr 23, 2024

Docker tags
docker.io/radixdlt/private-scrypto-builder:02c8b843c4

Copy link

github-actions bot commented Apr 23, 2024

Benchmark for 02c8b84

Click to view benchmark
Test Base PR %
costing::bench_prepare_wasm 65.8±0.18ms 65.8±0.12ms 0.00%
costing::decode_sbor 10.8±0.03µs 10.8±0.02µs 0.00%
costing::decode_sbor_bytes 29.2±0.08µs 29.6±0.01µs +1.37%
costing::deserialize_wasm 1302.4±19.43µs 1297.4±5.80µs -0.38%
costing::instantiate_flash_loan 5.7±0.05ms 4.0±0.75ms -29.82%
costing::instantiate_radiswap 5.6±0.05ms 5.6±0.06ms 0.00%
costing::spin_loop 21.5±0.03ms 21.7±0.10ms +0.93%
costing::validate_sbor_payload 28.9±0.08µs 30.1±0.04µs +4.15%
costing::validate_sbor_payload_bytes 235.4±0.57ns 253.5±1.90ns +7.69%
costing::validate_secp256k1 76.2±0.08µs 76.4±0.07µs +0.26%
costing::validate_wasm 37.1±0.07ms 36.5±0.05ms -1.62%
decimal::add/0 8.4±0.00ns 8.4±0.00ns 0.00%
decimal::add/rust-native 9.8±0.00ns 9.8±0.00ns 0.00%
decimal::add/wasmer 116.9±0.10ns 121.3±1.59ns +3.76%
decimal::add/wasmer-call-native 455.3±1.20ns 457.6±0.37ns +0.51%
decimal::add/wasmi 710.6±4.43ns 632.6±1.32ns -10.98%
decimal::add/wasmi-call-native 5.6±0.04µs 5.3±0.01µs -5.36%
decimal::div/0 191.4±0.26ns 191.0±0.14ns -0.21%
decimal::from_string/0 157.0±0.31ns 158.7±0.25ns +1.08%
decimal::mul/0 143.3±0.11ns 142.8±0.05ns -0.35%
decimal::mul/rust-native 138.1±0.11ns 134.7±0.20ns -2.46%
decimal::mul/wasmer 1519.2±3.34ns 1530.1±0.70ns +0.72%
decimal::mul/wasmer-call-native 580.2±0.51ns 592.5±0.34ns +2.12%
decimal::mul/wasmi 42.3±0.07µs 41.8±0.15µs -1.18%
decimal::mul/wasmi-call-native 5.7±0.00µs 5.5±0.02µs -3.51%
decimal::pow/0 656.2±0.49ns 656.3±0.27ns +0.02%
decimal::pow/rust-native 629.6±0.64ns 633.6±0.42ns +0.64%
decimal::pow/wasmer 6.6±0.00µs 6.6±0.01µs 0.00%
decimal::pow/wasmer-call-native 1029.1±1.34ns 1031.3±0.73ns +0.21%
decimal::pow/wasmi 197.4±0.35µs 195.7±0.28µs -0.86%
decimal::pow/wasmi-call-native 5.4±0.01µs 5.3±0.01µs -1.85%
decimal::root/0 7.7±0.01µs 7.7±0.01µs 0.00%
decimal::sub/0 8.5±0.01ns 8.5±0.01ns 0.00%
decimal::to_string/0 443.4±0.50ns 440.6±0.29ns -0.63%
precise_decimal::add/0 9.3±0.11ns 9.8±0.02ns +5.38%
precise_decimal::add/rust-native 11.4±0.00ns 11.4±0.00ns 0.00%
precise_decimal::add/wasmer 118.5±0.10ns 119.0±0.13ns +0.42%
precise_decimal::add/wasmer-call-native 487.9±0.28ns 490.8±0.24ns +0.59%
precise_decimal::add/wasmi 867.6±1.27ns 790.4±1.02ns -8.90%
precise_decimal::add/wasmi-call-native 7.3±0.02µs 6.9±0.01µs -5.48%
precise_decimal::div/0 303.3±0.17ns 300.5±0.31ns -0.92%
precise_decimal::from_string/0 197.6±0.18ns 195.2±0.19ns -1.21%
precise_decimal::mul/0 345.4±0.23ns 343.7±0.67ns -0.49%
precise_decimal::mul/rust-native 304.9±1.25ns 305.9±1.20ns +0.33%
precise_decimal::mul/wasmer 3.4±0.00µs 3.5±0.00µs +2.94%
precise_decimal::mul/wasmer-call-native 832.2±1.40ns 840.5±1.60ns +1.00%
precise_decimal::mul/wasmi 108.3±0.12µs 106.8±0.16µs -1.39%
precise_decimal::mul/wasmi-call-native 7.6±0.01µs 7.3±0.02µs -3.95%
precise_decimal::pow/0 1850.9±3.18ns 1847.5±3.22ns -0.18%
precise_decimal::pow/rust-native 1460.7±3.41ns 1474.2±2.79ns +0.92%
precise_decimal::pow/wasmer 16.1±0.01µs 16.2±0.01µs +0.62%
precise_decimal::pow/wasmer-call-native 2.1±0.00µs 2.1±0.00µs 0.00%
precise_decimal::pow/wasmi 526.6±0.38µs 514.6±1.35µs -2.28%
precise_decimal::pow/wasmi-call-native 13.5±0.04µs 13.3±0.03µs -1.48%
precise_decimal::root/0 55.7±0.03µs 56.9±0.03µs +2.15%
precise_decimal::sub/0 9.5±0.00ns 9.5±0.02ns 0.00%
precise_decimal::to_string/0 718.7±2.46ns 722.3±2.30ns +0.50%
schema::validate_payload 343.3±0.40µs 355.3±0.57µs +3.50%
transaction::radiswap 5.3±0.02ms 5.3±0.02ms 0.00%
transaction::transfer 1745.7±8.33µs 1747.4±6.85µs +0.10%
transaction_processing::prepare 2.2±0.00ms 2.3±0.00ms +4.55%
transaction_processing::prepare_and_decompile 6.1±0.02ms 6.1±0.04ms 0.00%
transaction_processing::prepare_and_decompile_and_recompile 23.9±0.71ms 24.2±1.25ms +1.26%
transaction_validation::validate_manifest 42.2±0.02µs 42.1±0.67µs -0.24%
transaction_validation::verify_bls_2KB 976.2±21.39µs 1073.9±111.32µs +10.01%
transaction_validation::verify_bls_32B 1066.8±101.92µs 1025.0±54.69µs -3.92%
transaction_validation::verify_ecdsa 74.3±0.10µs 74.2±0.05µs -0.13%
transaction_validation::verify_ed25519 54.6±0.07µs 55.3±0.18µs +1.28%

@dhedey dhedey changed the base branch from develop to tweak/versioned-macro-improvements April 24, 2024 09:28
@dhedey dhedey changed the base branch from tweak/versioned-macro-improvements to develop April 24, 2024 12:09
@@ -428,7 +428,7 @@ pub enum TypedMainModuleSubstateValue {
NonFungibleVault(NonFungibleVaultTypedSubstateValue),
ConsensusManager(ConsensusManagerTypedSubstateValue),
Validator(ValidatorTypedSubstateValue),
AccessController(AccessControllerTypedSubstateValue),
AccessController(AccessControllerV2TypedSubstateValue),
Copy link
Member

Choose a reason for hiding this comment

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

We need both AccessControllerV1 and AccessControllerV2?

Copy link
Member Author

Choose a reason for hiding this comment

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

I do not think that v1 is needed here since both v1 and v2 substates can be represented as AccessControllerV2TypedSubstateValue.

Copy link
Member

Choose a reason for hiding this comment

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

Can we decode the V1 sbor payload into AccessControllerV2TypedSubstateValue?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup should be possible since the main change is that we added a new version (enum variant) so we should be able to decode v1 as v2, I've just added a test for that: access_controller_state_v1_can_be_decoded_as_v2.

Also, this is implicitly tested in test_all_scenario_commit_receipts_should_have_substate_changes_which_can_be_typed since our scenarios use AccessControllerV1 and their state can be typed.

@@ -153,7 +153,7 @@ pub enum TypedMainModuleSubstateKey {
NonFungibleVault(NonFungibleVaultTypedSubstateKey),
ConsensusManager(ConsensusManagerTypedSubstateKey),
ValidatorField(ValidatorTypedSubstateKey),
AccessController(AccessControllerTypedSubstateKey),
AccessController(AccessControllerV2TypedSubstateKey),
Copy link
Member

Choose a reason for hiding this comment

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

If v1 and v2 are the same, we should keep AccessControllerTypedSubstateKey? or we should have AccessControllerV1 and AccessControllerV2?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that this is similar to the other discussion, I believe that blueprint updates that extend the state (adding fields as an example) we can use latest versions.

@0xOmarA 0xOmarA merged commit b0aa203 into develop Apr 29, 2024
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
files-moved native-blueprint-interface Any change to the interface of native blueprints. NOTE: Also tag with `manifest` and `scrypto-lib` scrypto-lib Any change to the scrypto library
Development

Successfully merging this pull request may close these issues.

4 participants