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

Basic account type #4021

Merged
merged 3 commits into from
Jan 4, 2017
Merged

Basic account type #4021

merged 3 commits into from
Jan 4, 2017

Conversation

rphmeier
Copy link
Contributor

@rphmeier rphmeier commented Jan 3, 2017

Just the decoded RLP in a strongly-typed structure. I have another usecase for this which will be in an upcoming PR.

Differs from PodAccount as it doesn't store the storage/data inline; rather, this represents a leaf of the state trie.

@rphmeier rphmeier added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Jan 3, 2017
@rphmeier rphmeier requested a review from tomusdrw January 3, 2017 16:06
Copy link
Collaborator

@tomusdrw tomusdrw 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 for me, except for pub fields grumble

#[derive(Debug, Clone, PartialEq, Eq)]
pub struct BasicAccount {
/// Nonce of the account.
pub nonce: U256,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you need to modify the account somewhere?
Wouldn't it be better to have access methods for this?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, I think it's the only way to create ACC_EMPTY, right? Still I don't really like that you can modify internals freely.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, we could also move ACC_EMPTY into this module. Either way, the type needs to be created somehow, whether with Rlp::decode or BasicAccount::new which would still allow you to alter the internals, just with a read-modify-write, while making it more complicated to instantiate/use in general.

@rphmeier rphmeier mentioned this pull request Jan 4, 2017
@rphmeier rphmeier merged commit 99f091e into master Jan 4, 2017
@rphmeier rphmeier deleted the basic-account branch January 4, 2017 18:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants