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

EEI: Account handle #112

Open
chfast opened this issue Jul 18, 2018 · 8 comments
Open

EEI: Account handle #112

chfast opened this issue Jul 18, 2018 · 8 comments

Comments

@chfast
Copy link
Collaborator

chfast commented Jul 18, 2018

The main issue with the cost of e.g. getBalance() is the fact that the account lookup in the database might be needed. This idea is to split the account loading from the accessing account metadata.

Instead of

getBalance(address, resultOffset);

we should have

handle = loadAccount(address);
getBalance(handle, resultOffset);

Similarly to getBalance() there should be getters for code hash, code, code size and others.

Pros

  1. The cost of account lookup is separated from the cost of the getter.
  2. Getting information about the current account should be cheap because it is already loaded (the handle to the current account could be predefined).
  3. The cost of accessing the same account multiple times is lower.

Cons

  1. Handles must be deterministic as they may leak. Simple solution would be to have an array of loaded accounts and return the index in this array. Each call to loadAccount would append new entry to the array.
  2. Contracts are responsible of accounts management.

Alternatives

  1. Just dump account matadata to memory: loadAccount(memoryOffset). This is simpler, but might waste some memory when contract is not interested in all data. Is is also not extensible, i.e. we cannot change the account representation in the future.

  2. Extension of the alternative 1 where the contract specify the bit mask of account fields it is interested in, e.g. loadAccount(BALANCE | NONCE, memoryOffset). This at least allows adding more fields to the account in the future. But the output would be a mess, especially when getting the account code is considered.

@lrettig
Copy link
Member

lrettig commented Jul 18, 2018

split the account loading from the accessing account metadata

Makes sense to me, and seems to better reflect the actual I/O cost of the operation.

@pepyakin
Copy link

This is simpler, but might waste some memory when contract is not interested in all data

is this really an issue? memory size is neglegible and the copy cost is dwarfed by the IO

Is is also not extensible, i.e. we cannot change the account representation in the future.

It can be mitigated easily though, by adding bitflags, integer or another versioning mechanism

@chfast
Copy link
Collaborator Author

chfast commented Jul 18, 2018

Is is also not extensible, i.e. we cannot change the account representation in the future.

It can be mitigated easily though, by adding bitflags, integer or another versioning mechanism

I believe that's not enough. If you deploy a contract that supports the current version 1 there is no way to support future version 2. Maybe the contract should specify what account fields it is interested in. I added this idea as the alternative #2.

@pepyakin
Copy link

I believe that's not enough. If you deploy a contract that supports the current version 1 there is no way to support future version 2.

Could you explain me why that matters? A contract is written against version 1 and explicitly requests v.1 and gets the result in v.1, that sounds good to me and I can't see a reason why it should care about v.2 or something (unless you want to do something like a dynamic proxy that requests and forwards some account data and while it should be forward-compatible).

Maybe the contract should specify what account fields it is interested in.

Yeah, that's what i meant by bitflags : )

@chfast
Copy link
Collaborator Author

chfast commented Jul 23, 2018

Could you explain me why that matters? A contract is written against version 1 and explicitly requests v.1 and gets the result in v.1, that sounds good to me and I can't see a reason why it should care about v.2 or something (unless you want to do something like a dynamic proxy that requests and forwards some account data and while it should be forward-compatible).

Yes, that will work. For that I'd rather suggest using different function names for new versions. E.g. loadAccountV2().

Previously, I though you meant to return the format version number as a part of the account structure.

@pepyakin
Copy link

Previously, I though you meant to return the format version number as a part of the account structure.

Ah sorry for that, I was writting on the go back then. Should have make this more clear

@chfast
Copy link
Collaborator Author

chfast commented Jul 23, 2018

@pepyakin do you have anything similar in pwasm?

@pepyakin
Copy link

Nope, we don't!

@axic axic added this to Proposed in Revision 4 Jul 30, 2018
@chfast chfast removed this from Proposed in Revision 4 Aug 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants