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

STUB: New transaction format #194

Closed
1 task done
pipermerriam opened this issue Dec 5, 2017 · 10 comments
Closed
1 task done

STUB: New transaction format #194

pipermerriam opened this issue Dec 5, 2017 · 10 comments
Assignees
Labels

Comments

@pipermerriam
Copy link
Member

pipermerriam commented Dec 5, 2017

See sharding doc

  • Update Transaction format (depend on Account Abstraction #203 and STUB: Account access restriction #192)
    The format of a transaction now becomes:
    [
        chain_id,      # 1 on mainnet
        shard_id,      # the shard the transaction goes onto
        target,        # account the tx goes to
        data,          # transaction data
        start_gas,     # starting gas
        gasprice,      # gasprice
        access_list,   # access list (see below for specification)
        code           # initcode of the target (for account creation)
    ]

- [ ] Update apply_transaction function procedure

@pipermerriam pipermerriam mentioned this issue Dec 5, 2017
15 tasks
@hwwhww hwwhww added the eth2.0 label Dec 5, 2017
@jannikluhn jannikluhn self-assigned this Dec 13, 2017
@jannikluhn
Copy link
Contributor

jannikluhn commented Dec 13, 2017

I'm gonna start working on this, but disregard any account abstraction stuff for the moment (as there's no clear/single specification yet). So the transaction object will include nonce and signature. Removing those should be trivial later.

@jannikluhn
Copy link
Contributor

Closing this with #223 (apply_transaction hasn't been updated yet, but this belongs more to #203 in my opinion)

@hwwhww
Copy link
Contributor

hwwhww commented Jan 8, 2018

@jannikluhn I will add a new ticket for apply_transaction with new transaction format.

@jannikluhn
Copy link
Contributor

@hwwhww This seems to be included in #238 already. I think it will need some updates concerning gas calculation / payment and maybe state access restrictions, but the basics seem to be there. Can you comment, @NIC619 ?

@NIC619
Copy link
Contributor

NIC619 commented Jan 9, 2018

I updated the execute_transaction with the new transaction format in #238.
Regarding gas calculation, intrinsic gas cost is updated to also includes size of tx.code. Should we include the cost of reading access list?
Regarding payment, gas cost is updated to be paid by tx.to.
Regarding state access restriction, it seems to met that it's already be done in #202 ?

@jannikluhn @hwwhww Please let me know if there's any update that's missing regarding the new transaction format.

@jannikluhn
Copy link
Contributor

@NIC619 I think the only thing wrong with the transaction format itself is that gas_price is still part of it (at least given https://ethresear.ch/t/tradeoffs-in-account-abstraction-proposals/263/23). Regarding transaction execution I can see three things which need to be changed long term, but are probably fine for now:

  • gas subtraction at the protocol level (this should be solved in Add PAYGAS opcode #234 I guess)
  • there should be a way to stop execution after N gas has been consumed unless PAYGAS has been called (depending on the chosen account abstraction proposal)
  • IMO the hardest one: There needs to be a way to construct the access list without prior knowledge. So there should be a execution mode that works with an empty access list (or a placeholder) and that keeps track of which storage slots are accessed. The problem here is that changing the access list potentially changes the code path (as it changes SIGHASH). Also, it may require signing the same transaction twice (with the empty and with the filled access list).

Should we include the cost of reading access list?

Good point, my guess is that with stateless clients gas costs of state access in general will need to be revised significantly.

@hwwhww
Copy link
Contributor

hwwhww commented Jan 9, 2018

@jannikluhn

There needs to be a way to construct the access list without prior knowledge. So there should be a execution mode that works with an empty access list (or a placeholder) and that keeps track of which storage slots are accessed. The problem here is that changing the access list potentially changes the code path (as it changes SIGHASH). Also, it may require signing the same transaction twice (with the empty and with the filled access list).

Could you open a new ticket to describe "constructing the access list"? Or reopen #192 for the whole access restriction mechanism? 🙂

Should we include the cost of reading access list?

Is it already be included in the cost of SLOAD and SSTORE implicitly?

@jannikluhn
Copy link
Contributor

jannikluhn commented Jan 9, 2018

Could you open a new ticket to describe "constructing the access list"? Or reopen #192 for the whole access restriction mechanism? 🙂

Will do.

Is it already be included in the cost of SLOAD and SSTORE implicitly?

Charging per access list entry would mean that accessing the same storage slot multiple times wouldn't cost more which is reasonable. On the other hand we can't discriminate between read and writes there. So a combination of both would probably make sense.

@NIC619
Copy link
Contributor

NIC619 commented Jan 10, 2018

@jannikluhn yes, gas_price will be removed in #234.

there should be a way to stop execution after N gas has been consumed unless PAYGAS has been called (depending on the chosen account abstraction proposal)

I think there would be something like check_PAYGAS_execution which basically acts like execute_transaction but with a gas limit(specified by miner?) and returns True if PAYGASopcode is execute or returnFalse` if gas limit reached.

IMO the hardest one: There needs to be a way to construct the access list without prior knowledge. So there should be a execution mode that works with an empty access list (or a placeholder) and that keeps track of which storage slots are accessed. The problem here is that changing the access list potentially changes the code path (as it changes SIGHASH). Also, it may require signing the same transaction twice (with the empty and with the filled access list).

@ jannikluhn Can you elaborate more on "The problem here is that changing the access list potentially changes the code path (as it changes SIGHASH)."? Sorry I didn't follow.
As for signing transaction twice, based on my understanding, signing transaction is not needed in Account Abstraction as transaction sender is always ENTRY_POINT? Or I probably have misunderstood something.

Is it already be included in the cost of SLOAD and SSTORE implicitly?

@hwwhww My understanding is that it will be charged twice. One as the cost of witness data and one as data operated by SLOAD/SSTORE.

@vbuterin
Copy link

vbuterin commented Jan 10, 2018

There needs to be a way to construct the access list without prior knowledge. So there should be a execution mode that works with an empty access list (or a placeholder) and that keeps track of which storage slots are accessed.

IMO doing this with an execution mode is a bad idea; it would break in any scenario where the keys accessed depend even slightly on variables that could be affected by third parties. The access lists should be generated by something which gets added to Viper; you would pass a function name and arguments and Viper would reply back with what addresses and storage keys could get read or written.

For now, I recommend that we stick to setting the access list to be the sender and receiver, including their entire storage, and possibly add a field for "manually add an additional address" for transactions to contracts that trigger payouts to third-party addresses; don't bother doing partial storage witnesses until later. Even sender and receiver only covers a huge number of use cases.

Good point, my guess is that with stateless clients gas costs of state access in general will need to be revised significantly.

Yep! My personal preference at the moment BTW is that we charge a clean 4 gas per byte for all transaction or witness data, and we compute witness sizes based on the trie/state root at the start of collation execution (ie. part of the witness verification step of verifying a collation).

Regarding cost of SLOAD and SSTORE, I recommend just keeping the status quo for now.

I think there would be something like check_PAYGAS_execution which basically acts like execute_transaction but with a gas limit(specified by miner?) and returns True if PAYGASopcode is execute or returnFalse` if gas limit reached.

I think the solution may be simpler. Rather than just returning success or failure, we could return two kinds of failure, one where PAYGAS is executed, and if so with what gasprice, and the other where PAYGAS is not executed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants