Replace GenericApplicationId with MultiAddress.#3593
Replace GenericApplicationId with MultiAddress.#3593deuszx wants to merge 1 commit intouserapplicationid-is-multiaddressfrom
Conversation
deuszx
force-pushed
the
userapplicationid-is-multiaddress
branch
2 times, most recently
from
133b8cb to
aed6cc2
Compare
deuszx
force-pushed
the
rm-multiaddress-chain
branch
2 times, most recently
from
16a74dc to
34767d0
Compare
afck
left a comment
afck
left a comment
There was a problem hiding this comment.
Would it make sense to just call the type Address? "Multi" sounds like it could be more than one address (as opposed to one of several different kinds of addresses).
|
|
||
| impl BcsHashable<'_> for Chain {} | ||
|
|
||
| /// The CryptoHash of `Chain` newtype struct. |
There was a problem hiding this comment.
| /// The CryptoHash of `Chain` newtype struct. | |
| /// The `CryptoHash` of `Chain` newtype struct. |
(It's also not ideal that the comment of a pub item refers to a private type.)
There was a problem hiding this comment.
I agree, but this will go away when we switch to Account::Reserved variant.
|
|
||
| impl MultiAddress { | ||
| /// Returns the default chain address. | ||
| pub fn chain() -> Self { |
There was a problem hiding this comment.
Could this be a pub const CHAIN instead?
There was a problem hiding this comment.
Unfortunately we cannot. I can't use const and call CryptoHash::new and I can't construct one manually.
There was a problem hiding this comment.
I prefer what's in our internal design doc: Reserved(0) (syntax 0x0) for the chain balance.
There was a problem hiding this comment.
I don't think we need a special variant for this. It doesn't seem like it's needed anywhere and I've been trying to get rid of unnecessary variants, I don't want to re-introduce them.
There was a problem hiding this comment.
When we have both Address32 and Address20, I don't like to choose between the two for special addresses
There was a problem hiding this comment.
The chain will always have one address and we can hardcode the choice. Let's discuss that in a meeting.
deuszx
force-pushed
the
userapplicationid-is-multiaddress
branch
4 times, most recently
from
a917cca to
b2f4ee3
Compare
deuszx
force-pushed
the
rm-multiaddress-chain
branch
from
34767d0 to
b6b4700
Compare
deuszx
force-pushed
the
userapplicationid-is-multiaddress
branch
from
b2f4ee3 to
864f74a
Compare
deuszx
force-pushed
the
rm-multiaddress-chain
branch
from
b6b4700 to
74195cc
Compare
deuszx
force-pushed
the
userapplicationid-is-multiaddress
branch
2 times, most recently
from
6dd49a2 to
cfe2cd5
Compare
deuszx
force-pushed
the
rm-multiaddress-chain
branch
from
74195cc to
17cc7e7
Compare
|
(Re)addressed in #3633 |
3 participants
Motivation
Simplify types, prepare for multiple variants of
Address.Proposal
Remove
GenericApplicationIdenum (which had two variants:SystemandUser(UserApplicationId)) and useMultiAddressdirectly.GenericApplicationId::Systemvariant is now using a specialMultiAddress::chain()address which is a special address used for system/chain operations (pub-sub channels, etc.) and itsCryptoHashcannot be constructed by the user.Test Plan
CI
Release Plan
Links