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

Final Call: ECIP-1054 Atlantis Upgrade #78

Closed
soc1c opened this issue May 23, 2019 · 16 comments
Closed

Final Call: ECIP-1054 Atlantis Upgrade #78

soc1c opened this issue May 23, 2019 · 16 comments

Comments

@soc1c
Copy link
Contributor

soc1c commented May 23, 2019

ETC Core Devs Call - Atlantis Finalization

When: Thursday, May 30, 2019, 3pm UTC, 60 minutes max.

Where: Ethereum Classic Discord https://discord.gg/dwxb6nf #ecips channel. Will use/create a voice channel ad hoc.

Agenda

  • Quick client teams check-in
    • Geth / Multi-Geth
    • Parity Ethereum
    • IOHK Mantis
  • Atlantis (ECIP-1054) is in "last call" state
    • ECIP-1054 needs to be either accepted or updated (or rejected)
    • discuss whether EIP-161 should be included or not @meowsbits @sorpaas
    • discuss any other EIP that might cause uncertainty
    • discuss timeline for the protocol upgrade
      • Morden Classic and Kotti Classic testnet (August?)
      • Ethereum Classic mainnet (September?)
  • Anything else related to Atlantis
  • Outlook: Agharta (ECIP-1056) if time permits
  • Outlook: Astor SHA3 testnet if time permits

Please comment to add items to the agenda

@antsankov
Copy link
Contributor

antsankov commented May 28, 2019

Would like to share progress on Astor SHA3 testnet

  • What has been done so far,
  • What are the next steps,
  • Community feedback

https://astor.host - home page
https://stats.astor.host - stats page
https://explore.astor.host - block explorer

@meowsbits
Copy link
Member

meowsbits commented May 28, 2019

Sorry @antsankov, although I think it's great you're making progress in that space, I must say I motion against any topics that aren't directly related to the Atlantis fork in question for this particular meeting.

@soc1c
Copy link
Contributor Author

soc1c commented May 29, 2019

@antsankov added it to the agenda (if time permits)

Note, this is tomorrow!

@realcodywburns
Copy link
Member

the dust clean up isn't a contract, iirc. it is a change in transaction logic where by if a transaction is sent to an account the results in it being empty, the account is pruned. logistically, sending an transfer of zero value to all the empty accounts killed them.

@stevanlohja
Copy link
Contributor

Does EIP-170 cause issues with existing large contracts on ETC?

if contract creation initialization returns data with length of more than 0x6000 (214 + 213) bytes, contract creation fails with an out of gas error.
Src: https://github.com/ethereum/EIPs/blob/master/EIPS/eip-170.md

@meowsbits
Copy link
Member

Introducing EIP170 as a feature that enables at and beyond a given block transition value (ie some block height n (the value for the proposed fork)) would not impact contracts deployed in blocks below that value. This is how go-ethereum and multi-geth handle this logic (below).

ret, err := run(evm, contract, nil, false)

	// check whether the max code size has been exceeded
	maxCodeSizeExceeded := evm.ChainConfig().IsEIP170F(evm.BlockNumber) && len(ret) > params.MaxCodeSize
	// if the contract creation ran successfully and no errors were returned
	// calculate the gas required to store the code. If the code could not
	// be stored due to not enough gas set an error and let it be handled
	// by the error checking condition below.
	if err == nil && !maxCodeSizeExceeded {
		createDataGas := uint64(len(ret)) * params.CreateDataGas
		if contract.UseGas(createDataGas) {
			evm.StateDB.SetCode(address, ret)
		} else {
			err = ErrCodeStoreOutOfGas
		}
	}

	// When an error was returned by the EVM or when setting the creation code
	// above we revert to the snapshot and consume any gas remaining. Additionally
	// when we're in homestead this also counts for code storage gas errors.
	if maxCodeSizeExceeded || (err != nil && (evm.ChainConfig().IsEIP2F(evm.BlockNumber) || err != ErrCodeStoreOutOfGas)) {
		evm.StateDB.RevertToSnapshot(snapshot)
		if err != errExecutionReverted {
			contract.UseGas(contract.Gas)
		}
	}
	// Assign err if contract code size exceeds the max while the err is still empty.
	if maxCodeSizeExceeded && err == nil {
		err = errMaxCodeSizeExceeded
	}
	if evm.vmConfig.Debug && evm.depth == 0 {
		evm.vmConfig.Tracer.CaptureEnd(ret, gas-contract.Gas, time.Since(start), err)
	}
return ret, address, contract.Gas, err

https://github.com/multi-geth/multi-geth/blob/master/core/vm/evm.go#L408

Parity:

	/// See main EthashParams docs.
	pub max_code_size: Option<Uint>,
	/// Maximum size of transaction RLP payload.
	pub max_transaction_size: Option<Uint>,
	/// See main EthashParams docs.
	pub max_code_size_transition: Option<Uint>,
/// Transaction permission contract address.

https://github.com/paritytech/parity-ethereum/blob/master/json/src/spec/params.rs#L111-L116

@meowsbits
Copy link
Member

@realcodywburns gotcha... is there a list of dead accounts that need to receive a killer transaction? is there a record anywhere of who handled this cleanup and how on the foundation chain?

or just

for (a in allTheAccounts) {
 eth.sendTransaction({from: me, to: a, value: 0});
}

?

@stevanlohja
Copy link
Contributor

stevanlohja commented May 30, 2019

Does EIP-170 cause issues with existing large contracts on ETC?

The question answered in the call and above thread. EIP-170 will not cause issues with existing large contracts but will enforce a limit post fork and this can be changed in the future if need be. There were 2 opposers of this EIP in the call, but far more express for it. To my understanding, these opposers can make their technical case in the next call.

@soc1c
Copy link
Contributor Author

soc1c commented May 30, 2019

@YazzyYaz could you post notes here?

@YazzyYaz
Copy link
Contributor

YazzyYaz commented May 30, 2019

Agenda: Determine whether Atlantis in accepted state or continue working.

EIP-161 should be included or not?

Isaac: concern about post-implementation. State-clearing process. Changes in EIP-161 makes it valid to remove dead accounts and frees up space on the machine.

Someone has to do something for this to happen.

Need to know which accounts need to be removed. Need more information about implementation.

Sorpaas: Foundation side. You can change each empty account to clear it out. Use Op-code to clear the empty account. Filter out all the accounts that are valid for clearing.

Looking for dead accounts. 0 nonce, 0 balance, 0 code for Contract addresses.

Query for them.

Any concerns to include it? It doesn’t affect transaction history.

Pros to include it: Technical Debt to resolve. Something we can remove from the machine. It also helps with testing.

Impossible to do Atlantis without including the EIP-161, even for security concerns.

Agreement on including EIP-161 reached.


Stev asked about EIP-170 for contract code size. It will only stop for contract creation size that’s large.

Gigantic contracts will get stopped. Increasing contract size limit shouldn’t affect immutability according to Sorpaas. Best to include it to stay 1:1 with Foundation chain, at least with the non-contentious EIPs.

Anthony brought up letting miner side decide what size of contract transaction to mine. Sorpaas explains it doesn’t just affect contract creation but also internet contract calls which would also return out-of-gas errors if it’s a large contract as by EIP-170.

Shane asks if we are set on hard limit on gas. No contract size limit at the moment in ETC. Right now you pay gas based on how much you want to store. There’s a soft limit that can be changed.

Undecided on EIP-170 due to uncertainty over having a contract limit.

Makes testing easier according to Sorpaas.

Zach is saying that if a contract creation costs more than the gas limit, miners can vote on higher gas limits in future blocks to accommodate for contract.

It doesn’t break compatibility.

Anthony against it because a hardfork breaks pre-described agreements but not as a soft fork.

We will decide in 2 weeks time for another vote on EIP-170.

@YazzyYaz
Copy link
Contributor

Rough draft, sorry @soc1c ^

@pyskell
Copy link
Member

pyskell commented May 30, 2019

My opposition is the following:

  1. These rules can simply be applied to transaction validation rather than block validation, making it a soft fork rather than a hard fork.
  2. In practical terms this will make no difference, miners will download one of the clients that has these types of transactions excluded by default and future blocks will not contain any such transactions.
  3. It's vitally important to stick to pre-agreed rules when they're defined. In this case gas limit is a rule that governs computational size of transactions and there was no bound on physical data/contract size.
  4. There's precedent for these types of things in other cryptocurrencies, example: bitcoin/bitcoin@6a4c196
  5. Whether this is a contract creation call or an internal contract call shouldn't matter, both types of transactions can simply be rejected from inclusion in a block if their data is greater than maxCodeSize (per geth's implementation). As per point 2 I believe most clients will update and enforce the soft fork.

Regardless I don't want to delay the Atlantis hard fork and apologize for the late comment.

@phyro
Copy link
Member

phyro commented May 30, 2019

@pyskell thanks for the explanation of the idea. I have a different kind of question regarding

It's vitally important to stick to pre-agreed rules when they're defined. In this case gas limit is a rule that governs computational size of transactions and there was no bound on physical data/contract size.

I'm wondering what happens if the initial design of Ethereum is 'flawed'. I personally believe that some things are 'flawed' (though I'm open to being convinced otherwise) like for example:

  1. I believe we should limit the block size to have a guarantee that the chain size is able to scale (on L1) in a slow enough way so that majority can actually run it for the next decades

  2. I also think that having 15 second blocks isn't really beneficial for a store of value coin because you will always need more than 1 confirmation (reasoning from the fact that security is a function of time & energy, not the number of blocks). So we're basically just adding block headers for no good reason - I could be missing stuff here (actually there is one benefit that I know of, which is that you can gather the statistical data on the fly about things included in the block header, but I don't think this is used yet)

Do we say these rules are pre-agreed and that we MUST stick to them?

Btw, regarding the proposal from pyskell. I'm still slightly more in favor for a hard cap, but we could try the soft fork solution to see if it catches on and we could do a hard cap later on if needed. Assuming pyskell solution has no attack vectors and that the miners do adopt it, how much harder does this make it for devs?

@soc1c
Copy link
Contributor Author

soc1c commented May 31, 2019

To be continued with EIP-170 in #79

@soc1c soc1c closed this as completed May 31, 2019
@soc1c
Copy link
Contributor Author

soc1c commented May 31, 2019

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

No branches or pull requests

8 participants