-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Make Chain a variant of AccountOwner #3560
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
Changes from all commits
7e2fb52
bc2626b
85f269f
5c2e46b
0a97ae8
b6110b9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -37,7 +37,9 @@ pub enum AccountOwner { | |
| /// An account owned by a user. | ||
| User(Owner), | ||
| /// An account for an application. | ||
| Application(ApplicationId), | ||
| Application(UserApplicationId), | ||
| /// Chain account. | ||
| Chain, | ||
| } | ||
|
|
||
| /// A system account. | ||
|
|
@@ -48,34 +50,30 @@ pub struct Account { | |
| /// The chain of the account. | ||
| pub chain_id: ChainId, | ||
| /// The owner of the account, or `None` for the chain balance. | ||
| #[debug(skip_if = Option::is_none)] | ||
| pub owner: Option<AccountOwner>, | ||
| pub owner: AccountOwner, | ||
| } | ||
|
|
||
| impl Account { | ||
| /// Creates an [`Account`] representing the balance shared by a chain's owners. | ||
| pub fn chain(chain_id: ChainId) -> Self { | ||
| Account { | ||
| chain_id, | ||
| owner: None, | ||
| owner: AccountOwner::Chain, | ||
| } | ||
| } | ||
|
|
||
| /// Creates an [`Account`] for a specific [`Owner`] on a chain. | ||
| pub fn owner(chain_id: ChainId, owner: impl Into<AccountOwner>) -> Self { | ||
| Account { | ||
| chain_id, | ||
| owner: Some(owner.into()), | ||
| owner: owner.into(), | ||
| } | ||
| } | ||
| } | ||
|
|
||
| impl Display for Account { | ||
| fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { | ||
| match self.owner { | ||
| Some(owner) => write!(f, "{}:{}", self.chain_id, owner), | ||
| None => write!(f, "{}", self.chain_id), | ||
| } | ||
| write!(f, "{}:{}", self.chain_id, self.owner) | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -306,7 +304,7 @@ pub struct ApplicationId<A = ()> { | |
|
|
||
| /// Alias for `ApplicationId`. Use this alias in the core | ||
| /// protocol where the distinction with the more general enum `GenericApplicationId` matters. | ||
| pub type UserApplicationId<A = ()> = ApplicationId<A>; | ||
| pub type UserApplicationId = ApplicationId<()>; | ||
ma2bd marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| /// A unique identifier for an application. | ||
| #[derive( | ||
|
|
@@ -328,7 +326,7 @@ pub enum GenericApplicationId { | |
| /// The system application. | ||
| System, | ||
| /// A user application. | ||
| User(ApplicationId), | ||
| User(UserApplicationId), | ||
| } | ||
|
|
||
| impl GenericApplicationId { | ||
|
|
@@ -951,6 +949,7 @@ impl<'de> serde::de::Visitor<'de> for OwnerVisitor { | |
| enum SerializableAccountOwner { | ||
| User(Owner), | ||
| Application(ApplicationId), | ||
| Chain, | ||
| } | ||
|
|
||
| impl Serialize for AccountOwner { | ||
|
|
@@ -961,6 +960,7 @@ impl Serialize for AccountOwner { | |
| match self { | ||
| AccountOwner::Application(app_id) => SerializableAccountOwner::Application(*app_id), | ||
| AccountOwner::User(owner) => SerializableAccountOwner::User(*owner), | ||
| AccountOwner::Chain => SerializableAccountOwner::Chain, | ||
| } | ||
| .serialize(serializer) | ||
| } | ||
|
|
@@ -980,6 +980,7 @@ impl<'de> Deserialize<'de> for AccountOwner { | |
| Ok(AccountOwner::Application(app_id)) | ||
| } | ||
| SerializableAccountOwner::User(owner) => Ok(AccountOwner::User(owner)), | ||
| SerializableAccountOwner::Chain => Ok(AccountOwner::Chain), | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -990,6 +991,7 @@ impl Display for AccountOwner { | |
| match self { | ||
| AccountOwner::User(owner) => write!(f, "User:{}", owner)?, | ||
| AccountOwner::Application(app_id) => write!(f, "Application:{}", app_id)?, | ||
| AccountOwner::Chain => write!(f, "Chain")?, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I look forward to removing these tags
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The
The first one isn't just a refactor (internal change) as it requires making some high-level design decisions around opening new chains etc. I think the second option is the easier one and we should try to do it. We could hash a string like
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Addresses are not public keys but hash values, so |
||
| }; | ||
|
|
||
| Ok(()) | ||
|
|
@@ -1008,6 +1010,8 @@ impl FromStr for AccountOwner { | |
| Ok(AccountOwner::Application( | ||
| ApplicationId::from_str(app_id).context("Getting ApplicationId should not fail")?, | ||
| )) | ||
| } else if s.strip_prefix("Chain").is_some() { | ||
| Ok(AccountOwner::Chain) | ||
| } else { | ||
| Err(anyhow!("Invalid enum! Enum: {}", s)) | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -219,9 +219,14 @@ where | |
| if query.request_committees { | ||
| info.requested_committees = Some(chain.execution_state.system.committees.get().clone()); | ||
| } | ||
| if let Some(owner) = query.request_owner_balance { | ||
| info.requested_owner_balance = | ||
| chain.execution_state.system.balances.get(&owner).await?; | ||
| match query.request_owner_balance { | ||
| owner @ AccountOwner::Application(_) | owner @ AccountOwner::User(_) => { | ||
| info.requested_owner_balance = | ||
| chain.execution_state.system.balances.get(&owner).await?; | ||
| } | ||
| AccountOwner::Chain => { | ||
| info.requested_owner_balance = Some(*chain.execution_state.system.balance.get()); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here or very soon we should remove
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I took a quick stab at removing the chains' balances entirely and it wasn't very difficult. I can propose something in a separate PR. |
||
| } | ||
| } | ||
| if let Some(next_block_height) = query.test_next_block_height { | ||
| ensure!( | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should fix that (not here but soon)