Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Customizable ink address #10521

Merged

Conversation

kvinwang
Copy link
Contributor

@kvinwang kvinwang commented Dec 21, 2021

Background

We are using the pallet-contracts to implement phala's smart contract. We constructed a minimal substrate runtime which include only pallet-contracts and it's dependencies. The runtime then can runs on multiple seperate storage instance. The problem is that contracts instantiated in different storage instance can have the same address, and we want the address to be unique in global.

Summary

To solve the problem, this PR adds a type parameter to generate ink addresses. The DefaultAddressGenerator keeps the original formula. And users can customize the address generation formula by implement a custom AddressGenerator.

@kvinwang kvinwang requested a review from athei as a code owner December 21, 2021 09:15
Copy link
Member

@athei athei left a comment

Choose a reason for hiding this comment

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

Looks good. Just missing rust doc.

@@ -136,6 +136,43 @@ type BalanceOf<T> =
/// The current storage version.
const STORAGE_VERSION: StorageVersion = StorageVersion::new(6);

/// The address generator function used to determine the address of a contract.
pub trait AddressGenerator<T: frame_system::Config> {
fn generate_address(
Copy link
Member

Choose a reason for hiding this comment

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

Every public item needs rust doc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

Copy link
Member

Choose a reason for hiding this comment

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

Thanks. I think we need to document some warnings here though:

  1. Implementer must make sure that there are no collisions (same output for different inputs).
  2. Implementer must make sure that the same inputs lead to the same output (including environmental inputs like your storage location).
  3. Changing the implementation through a runtime upgrade without a proper storage migration would be a a catastrophic event.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the suggestion. Appended them to the doc.

@h4x3rotab
Copy link
Contributor

LGTM

@athei athei added A0-please_review Pull request needs code review. B3-apinoteworthy C1-low PR touches the given topic and has a low impact on builders. D2-notlive 💤 PR contains changes in a runtime directory that is not deployed to a chain that requires an audit. labels Dec 22, 2021
@athei
Copy link
Member

athei commented Dec 23, 2021

bot merge

@paritytech-processbot paritytech-processbot bot merged commit 0444ab7 into paritytech:master Dec 23, 2021
@h4x3rotab h4x3rotab deleted the custom-ink-addr branch December 24, 2021 05:45
HCastano added a commit to paritytech/canvas that referenced this pull request Jan 8, 2022
cmichi pushed a commit to paritytech/canvas that referenced this pull request Jan 10, 2022
* Bump Cumulus, Polkadot, and Substrate

Also bumps some other depenencies

* Remove duplicate `polkadot` dependency

* Update `service.rs`

Changes related to: paritytech/cumulus#835

* Update `command.rs`

* Add `AddressGenerator` config type

From: paritytech/substrate#10521

* Allow Root to execute overweight XCMP messages

From: paritytech/cumulus#799

* Add `header` argument to `collect_collation_info`

From: paritytech/cumulus#882

* Update Cumulus and friends again

* Add Fork ID to genesis config

paritytech/cumulus#870

* Add `state_version` field

paritytech/substrate#9732

* Add `MaxConsumers` config parameter

paritytech/substrate#10382

* Update Substrate compatibility note in README
grishasobol pushed a commit to gear-tech/substrate that referenced this pull request Mar 28, 2022
ark0f pushed a commit to gear-tech/substrate that referenced this pull request Feb 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. C1-low PR touches the given topic and has a low impact on builders. D2-notlive 💤 PR contains changes in a runtime directory that is not deployed to a chain that requires an audit.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants