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

Add DAO support #843

Merged
merged 6 commits into from
Aug 27, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/vm-pr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ jobs:
# This is the most fair division among 1 directory vs. everything else
args: ['--excludeDir=stTimeConsuming', '--dir=GeneralStateTests/stTimeConsuming']
# Run specific fork tests
fork: ['MuirGlacier', 'Istanbul', 'Homestead', 'Chainstart']
fork: ['MuirGlacier', 'Istanbul', 'Homestead', 'Chainstart', 'dao']
Copy link
Member

Choose a reason for hiding this comment

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

Does Chaintstart refer to Frontier ("the genesis") here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes.

Copy link
Member

Choose a reason for hiding this comment

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

Yes.

fail-fast: false
steps:
- uses: actions/setup-node@v1
Expand Down
25 changes: 25 additions & 0 deletions packages/block/src/header.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,8 @@ export class BlockHeader {
},
]
defineProperties(this, fields, data)

this._checkDAOExtraData()
}

/**
Expand Down Expand Up @@ -357,4 +359,27 @@ export class BlockHeader {
return undefined
}
}

/**
* Force extra data be DAO_ExtraData for DAO_ForceExtraDataRange blocks after DAO
* activation block (see: https://blog.slock.it/hard-fork-specification-24b889e70703)
*/
private _checkDAOExtraData() {
const DAO_ExtraData = Buffer.from('64616f2d686172642d666f726b', 'hex')
const DAO_ForceExtraDataRange = 9

if (this._common.hardforkIsActiveOnChain('dao')) {
// verify the extraData field.
const blockNumber = new BN(this.number)
const DAOActivationBlock = new BN(this._common.hardforkBlock('dao'))
if (blockNumber.gte(DAOActivationBlock)) {
const drift = blockNumber.sub(DAOActivationBlock)
if (drift.lten(DAO_ForceExtraDataRange)) {
if (!this.extraData.equals(DAO_ExtraData)) {
throw new Error("extraData should be 'dao-hard-fork'")
}
}
}
}
}
}
22 changes: 22 additions & 0 deletions packages/block/test/block.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,4 +144,26 @@ tape('[Block]: block functions', function (t) {
st.equal(typeof block.toJSON(true), 'object')
st.end()
})

t.test('DAO hardfork', function (st) {
const blockData: any = rlp.decode(testData2.blocks[0].rlp)
// Set block number from test block to mainnet DAO fork block 1920000
blockData[0][8] = Buffer.from('1D4C00', 'hex')

const common = new Common('mainnet', 'dao')
st.throws(
function () {
new Block(blockData, { common: common })
},
/Error: extraData should be 'dao-hard-fork'$/,
'should throw on DAO HF block with wrong extra data',
) // eslint-disable-line

// Set extraData to dao-hard-fork
blockData[0][12] = Buffer.from('64616f2d686172642d666f726b', 'hex')
st.doesNotThrow(function () {
new Block(blockData, { common: common })
}, 'should not throw on DAO HF block with correct extra data')
st.end()
})
})
2 changes: 1 addition & 1 deletion packages/common/src/chains/goerli.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
},
{
"name": "dao",
"block": 0,
"block": null,
"forkHash": "0xa3f5ab08"
Copy link
Member

Choose a reason for hiding this comment

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

So I do get this right that DAO support is not active neither on Goerli nor on Kovan, correct?

Then this change makes a lot of sense and is technically also a bug fix along the way. Wonder if we should generally remove all the null DAO entries from Common completely since these are just HFs not being present on the respective chains (and never will be).

For null I always had the semantics "HF didn't happen yet but has some settings and will happen some time in the future" in mind, so e.g. the state of the Berlin HF right now. Technically it shouldn't make any difference though.

Copy link
Member Author

Choose a reason for hiding this comment

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

I do get the confusion here. But just for comparision:

geth: config="{ChainID: 1 Homestead: 1150000 DAO: 1920000 DAOSupport: true (...)

geth --goerli: config="{ChainID: 5 Homestead: 0 DAO: <nil> DAOSupport: true (...)

I just checked all 4 testnets and neither of them has any contract/code at "bf4ed7b27f1d666546e30d74d50d173d20bca754" (the DAO withdraw contract on mainnet)

This is thus also reflected in all 4 testnet common files now (they all have the DAO block set to null)

},
{
Expand Down
2 changes: 1 addition & 1 deletion packages/common/src/chains/kovan.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
},
{
"name": "dao",
"block": 0,
"block": null,
"forkHash": "0x10ffe56"
},
{
Expand Down
4 changes: 2 additions & 2 deletions packages/common/tests/hardforks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,8 @@ tape('[Common]: Hardfork logic', function (t: tape.Test) {
st.equal(c.activeHardforks().length, 8, msg)

c = new Common('goerli')
msg = 'should return 9 active HFs for goerli'
st.equal(c.activeHardforks().length, 9, msg)
msg = 'should return 8 active HFs for goerli'
st.equal(c.activeHardforks().length, 8, msg)

st.end()
})
Expand Down
121 changes: 121 additions & 0 deletions packages/vm/lib/config/dao_fork_accounts_config.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
{
"DAOAccounts": [
"d4fe7bc31cedb7bfb8a345f31e668033056b2728",
"b3fb0e5aba0e20e5c49d252dfd30e102b171a425",
"2c19c7f9ae8b751e37aeb2d93a699722395ae18f",
"ecd135fa4f61a655311e86238c92adcd779555d2",
"1975bd06d486162d5dc297798dfc41edd5d160a7",
"a3acf3a1e16b1d7c315e23510fdd7847b48234f6",
"319f70bab6845585f412ec7724b744fec6095c85",
"06706dd3f2c9abf0a21ddcc6941d9b86f0596936",
"5c8536898fbb74fc7445814902fd08422eac56d0",
"6966ab0d485353095148a2155858910e0965b6f9",
"779543a0491a837ca36ce8c635d6154e3c4911a6",
"2a5ed960395e2a49b1c758cef4aa15213cfd874c",
"5c6e67ccd5849c0d29219c4f95f1a7a93b3f5dc5",
"9c50426be05db97f5d64fc54bf89eff947f0a321",
"200450f06520bdd6c527622a273333384d870efb",
"be8539bfe837b67d1282b2b1d61c3f723966f049",
"6b0c4d41ba9ab8d8cfb5d379c69a612f2ced8ecb",
"f1385fb24aad0cd7432824085e42aff90886fef5",
"d1ac8b1ef1b69ff51d1d401a476e7e612414f091",
"8163e7fb499e90f8544ea62bbf80d21cd26d9efd",
"51e0ddd9998364a2eb38588679f0d2c42653e4a6",
"627a0a960c079c21c34f7612d5d230e01b4ad4c7",
"f0b1aa0eb660754448a7937c022e30aa692fe0c5",
"24c4d950dfd4dd1902bbed3508144a54542bba94",
"9f27daea7aca0aa0446220b98d028715e3bc803d",
"a5dc5acd6a7968a4554d89d65e59b7fd3bff0f90",
"d9aef3a1e38a39c16b31d1ace71bca8ef58d315b",
"63ed5a272de2f6d968408b4acb9024f4cc208ebf",
"6f6704e5a10332af6672e50b3d9754dc460dfa4d",
"77ca7b50b6cd7e2f3fa008e24ab793fd56cb15f6",
"492ea3bb0f3315521c31f273e565b868fc090f17",
"0ff30d6de14a8224aa97b78aea5388d1c51c1f00",
"9ea779f907f0b315b364b0cfc39a0fde5b02a416",
"ceaeb481747ca6c540a000c1f3641f8cef161fa7",
"cc34673c6c40e791051898567a1222daf90be287",
"579a80d909f346fbfb1189493f521d7f48d52238",
"e308bd1ac5fda103967359b2712dd89deffb7973",
"4cb31628079fb14e4bc3cd5e30c2f7489b00960c",
"ac1ecab32727358dba8962a0f3b261731aad9723",
"4fd6ace747f06ece9c49699c7cabc62d02211f75",
"440c59b325d2997a134c2c7c60a8c61611212bad",
"4486a3d68fac6967006d7a517b889fd3f98c102b",
"9c15b54878ba618f494b38f0ae7443db6af648ba",
"27b137a85656544b1ccb5a0f2e561a5703c6a68f",
"21c7fdb9ed8d291d79ffd82eb2c4356ec0d81241",
"23b75c2f6791eef49c69684db4c6c1f93bf49a50",
"1ca6abd14d30affe533b24d7a21bff4c2d5e1f3b",
"b9637156d330c0d605a791f1c31ba5890582fe1c",
"6131c42fa982e56929107413a9d526fd99405560",
"1591fc0f688c81fbeb17f5426a162a7024d430c2",
"542a9515200d14b68e934e9830d91645a980dd7a",
"c4bbd073882dd2add2424cf47d35213405b01324",
"782495b7b3355efb2833d56ecb34dc22ad7dfcc4",
"58b95c9a9d5d26825e70a82b6adb139d3fd829eb",
"3ba4d81db016dc2890c81f3acec2454bff5aada5",
"b52042c8ca3f8aa246fa79c3feaa3d959347c0ab",
"e4ae1efdfc53b73893af49113d8694a057b9c0d1",
"3c02a7bc0391e86d91b7d144e61c2c01a25a79c5",
"0737a6b837f97f46ebade41b9bc3e1c509c85c53",
"97f43a37f595ab5dd318fb46e7a155eae057317a",
"52c5317c848ba20c7504cb2c8052abd1fde29d03",
"4863226780fe7c0356454236d3b1c8792785748d",
"5d2b2e6fcbe3b11d26b525e085ff818dae332479",
"5f9f3392e9f62f63b8eac0beb55541fc8627f42c",
"057b56736d32b86616a10f619859c6cd6f59092a",
"9aa008f65de0b923a2a4f02012ad034a5e2e2192",
"304a554a310c7e546dfe434669c62820b7d83490",
"914d1b8b43e92723e64fd0a06f5bdb8dd9b10c79",
"4deb0033bb26bc534b197e61d19e0733e5679784",
"07f5c1e1bc2c93e0402f23341973a0e043f7bf8a",
"35a051a0010aba705c9008d7a7eff6fb88f6ea7b",
"4fa802324e929786dbda3b8820dc7834e9134a2a",
"9da397b9e80755301a3b32173283a91c0ef6c87e",
"8d9edb3054ce5c5774a420ac37ebae0ac02343c6",
"0101f3be8ebb4bbd39a2e3b9a3639d4259832fd9",
"5dc28b15dffed94048d73806ce4b7a4612a1d48f",
"bcf899e6c7d9d5a215ab1e3444c86806fa854c76",
"12e626b0eebfe86a56d633b9864e389b45dcb260",
"a2f1ccba9395d7fcb155bba8bc92db9bafaeade7",
"ec8e57756626fdc07c63ad2eafbd28d08e7b0ca5",
"d164b088bd9108b60d0ca3751da4bceb207b0782",
"6231b6d0d5e77fe001c2a460bd9584fee60d409b",
"1cba23d343a983e9b5cfd19496b9a9701ada385f",
"a82f360a8d3455c5c41366975bde739c37bfeb8a",
"9fcd2deaff372a39cc679d5c5e4de7bafb0b1339",
"005f5cee7a43331d5a3d3eec71305925a62f34b6",
"0e0da70933f4c7849fc0d203f5d1d43b9ae4532d",
"d131637d5275fd1a68a3200f4ad25c71a2a9522e",
"bc07118b9ac290e4622f5e77a0853539789effbe",
"47e7aa56d6bdf3f36be34619660de61275420af8",
"acd87e28b0c9d1254e868b81cba4cc20d9a32225",
"adf80daec7ba8dcf15392f1ac611fff65d94f880",
"5524c55fb03cf21f549444ccbecb664d0acad706",
"40b803a9abce16f50f36a77ba41180eb90023925",
"fe24cdd8648121a43a7c86d289be4dd2951ed49f",
"17802f43a0137c506ba92291391a8a8f207f487d",
"253488078a4edf4d6f42f113d1e62836a942cf1a",
"86af3e9626fce1957c82e88cbf04ddf3a2ed7915",
"b136707642a4ea12fb4bae820f03d2562ebff487",
"dbe9b615a3ae8709af8b93336ce9b477e4ac0940",
"f14c14075d6c4ed84b86798af0956deef67365b5",
"ca544e5c4687d109611d0f8f928b53a25af72448",
"aeeb8ff27288bdabc0fa5ebb731b6f409507516c",
"cbb9d3703e651b0d496cdefb8b92c25aeb2171f7",
"6d87578288b6cb5549d5076a207456a1f6a63dc0",
"b2c6f0dfbb716ac562e2d85d6cb2f8d5ee87603e",
"accc230e8a6e5be9160b8cdf2864dd2a001c28b6",
"2b3455ec7fedf16e646268bf88846bd7a2319bb2",
"4613f3bca5c44ea06337a9e439fbc6d42e501d0a",
"d343b217de44030afaa275f54d31a9317c7f441e",
"84ef4b2357079cd7a7c69fd7a37cd0609a679106",
"da2fef9e4a3230988ff17df2165440f37e8b1708",
"f4c64518ea10f995918a454158c6b61407ea345c",
"7602b46df5390e432ef1c307d4f2c9ff6d65cc97",
"bb9bc244d798123fde783fcc1c72d3bb8c189413",
"807640a13483f8ac783c557fcdf27be11ea4ac7a"
],
"DAORefundContract": "bf4ed7b27f1d666546e30d74d50d173d20bca754"
}
1 change: 1 addition & 0 deletions packages/vm/lib/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ export default class VM extends AsyncEventEmitter {
const supportedHardforks = [
'chainstart',
'homestead',
'dao',
'tangerineWhistle',
'spuriousDragon',
'byzantium',
Expand Down
39 changes: 39 additions & 0 deletions packages/vm/lib/runBlock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,14 @@ import VM from './index'
import Bloom from './bloom'
import { RunTxResult } from './runTx'
import { StateManager } from './state/index'
import Account from '@ethereumjs/account'

import * as DAOConfig from './config/dao_fork_accounts_config.json'

/* DAO account list */

const DAOAccountList = DAOConfig.DAOAccounts
const DAORefundContract = DAOConfig.DAORefundContract

/**
* Options for running a block.
Expand Down Expand Up @@ -125,6 +133,14 @@ export default async function runBlock(this: VM, opts: RunBlockOpts): Promise<Ru
await state.setStateRoot(opts.root)
}

// check for DAO support and if we should apply the DAO fork
if (
this._common.hardforkIsActiveOnChain('dao') &&
new BN(opts.block.header.number).eq(new BN(this._common.hardforkBlock('dao')))
) {
await _applyDAOHardfork(state)
}

Copy link
Member

Choose a reason for hiding this comment

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

Since this is such specialized code not very relevant in the very most reads of the source file I would tend to put this into a separate function _applyDAOHardfork() and extract from runBlock(), maybe after the two if checks (so "if active & block correct", maybe these can be generally combined into one &&-concatenated check to avoid the deeper indentation level).

// Checkpoint state
await state.checkpoint()
let result
Expand Down Expand Up @@ -321,3 +337,26 @@ async function rewardAccount(state: StateManager, address: Buffer, reward: BN):
account.balance = toBuffer(new BN(account.balance).add(reward))
await state.putAccount(address, account)
}

// apply the DAO fork changes to the VM
async function _applyDAOHardfork(state: StateManager) {
const DAORefundContractAddress = Buffer.from(DAORefundContract, 'hex')
if (!state.accountExists(DAORefundContractAddress)) {
await state.putAccount(DAORefundContractAddress, new Account())
}
const DAORefundAccount = await state.getAccount(DAORefundContractAddress)
let DAOBalance = new BN(DAORefundAccount.balance)

for (let address of DAOAccountList) {
// retrieve the account and add it to the DAO's Refund accounts' balance.
let account = await state.getAccount(Buffer.from(address, 'hex'))
DAOBalance.iadd(new BN(account.balance))
// clear the accounts' balance
account.balance = Buffer.alloc(0)
Copy link
Member

Choose a reason for hiding this comment

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

I will likely never learn these kind of things 🤨 - they just seem to not fit in my head - but is it clear here that setting the account balance to zero is done with the empty Buffer ([Buffer ]) and not the zero Buffer ([Buffer 00]))?

Will not make it a blocker though since transition test runs from ethereum/tests seem to indicate that this is correct and leads to the correct transition state roots.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is worth checking out at some point if we would handle it correctly if we put a 1-byte zero-byte (😅 ). However to calculate the state root the actual Buffer should be empty and not Buffer.from('00','hex') (1 byte length) otherwise state root calculation gets messed up.

await state.putAccount(Buffer.from(address, 'hex'), account)
}

// finally, put the Refund Account
DAORefundAccount.balance = toBuffer(DAOBalance)
await state.putAccount(DAORefundContractAddress, DAORefundAccount)
}
4 changes: 2 additions & 2 deletions packages/vm/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,12 @@
"coverage:test": "tape './tests/api/**/*.js' ./tests/tester.js --state --dist",
"docs:build": "typedoc --options typedoc.js",
"test:state": "ts-node ./tests/tester --state",
"test:state:allForks": "echo 'Homestead TangerineWhistle SpuriousDragon Byzantium Constantinople Petersburg Istanbul MuirGlacier' | xargs -n1 | xargs -I v1 npm run test:state -- --fork=v1",
"test:state:allForks": "echo 'Chainstart Homestead dao TangerineWhistle SpuriousDragon Byzantium Constantinople Petersburg Istanbul MuirGlacier' | xargs -n1 | xargs -I v1 npm run test:state -- --fork=v1",
Copy link
Member

Choose a reason for hiding this comment

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

The convention here for the HF names is normally to apply camel case, so at least start with a capital letter (Dao) (so but for dao it is a bit unclear to admit, could also be DAO). We shouldn't deviate on such things too much since these kind of things always have the potential to lead to some not-wanted side effects at some point.

Won't make this a review blocker though.

"test:state:selectedForks": "echo 'Homestead TangerineWhistle SpuriousDragon Petersburg' | xargs -n1 | xargs -I v1 npm run test:state -- --fork=v1",
"test:state:slow": "npm run test:state -- --runSkipped=slow",
"test:buildIntegrity": "npm run test:state -- --test='stackOverflow'",
"test:blockchain": "node -r ts-node/register --stack-size=1500 ./tests/tester --blockchain",
"test:blockchain:allForks": "echo 'Homestead TangerineWhistle SpuriousDragon Byzantium Constantinople Petersburg Istanbul MuirGlacier' | xargs -n1 | xargs -I v1 node -r ts-node/register --stack-size=1500 ./tests/tester --blockchain --fork=v1",
"test:blockchain:allForks": "echo 'Chainstart Homestead dao TangerineWhistle SpuriousDragon Byzantium Constantinople Petersburg Istanbul MuirGlacier' | xargs -n1 | xargs -I v1 node -r ts-node/register --stack-size=1500 ./tests/tester --blockchain --fork=v1",
"test:API": "tape -r ts-node/register --stack-size=1500 ./tests/api/**/*.js",
"test:API:browser": "npm run build && karma start karma.conf.js",
"test": "echo \"[INFO] Generic test cmd not used. See package.json for more specific test run cmds.\"",
Expand Down
22 changes: 12 additions & 10 deletions packages/vm/tests/BlockchainTestsRunner.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
const { setupPreConditions, verifyPostConditions } = require('./util.js')
const { setupPreConditions, verifyPostConditions, getDAOCommon } = require('./util.js')
const { addHexPrefix } = require('ethereumjs-util')
const Trie = require('merkle-patricia-tree').SecureTrie
const { Block, BlockHeader } = require('@ethereumjs/block')
const Blockchain = require('@ethereumjs/blockchain').default
const Common = require('@ethereumjs/common').default
const level = require('level')
const levelMem = require('level-mem')

module.exports = async function runBlockchainTest(options, testData, t) {
// ensure that the test data is the right fork data

if (testData.network != options.forkConfigTestSuite) {
t.comment('skipping test: no data available for ' + options.forkConfigTestSuite)
return
Expand All @@ -24,11 +26,11 @@ module.exports = async function runBlockchainTest(options, testData, t) {
validate = true
}

const hardfork = options.forkConfigVM

const common = (options.forkConfigTestSuite == "HomesteadToDaoAt5") ? getDAOCommon(5) : new Common('mainnet', options.forkConfigVM)
Copy link
Member

Choose a reason for hiding this comment

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

Ah, nice. 😄

const blockchain = new Blockchain({
db: blockchainDB,
hardfork,
common,
validateBlocks: validate,
validatePow: validate,
})
Expand All @@ -43,20 +45,21 @@ module.exports = async function runBlockchainTest(options, testData, t) {
} else {
VM = require('../lib/index').default
}

const vm = new VM({
state,
blockchain,
hardfork,
common,
})

const genesisBlock = new Block(undefined, { hardfork })
const genesisBlock = new Block(undefined, { common })

// set up pre-state
await setupPreConditions(vm.stateManager._trie, testData)

// create and add genesis block
genesisBlock.header = new BlockHeader(formatBlockHeader(testData.genesisBlockHeader), {
hardfork,
common,
})

t.ok(vm.stateManager._trie.root.equals(genesisBlock.header.stateRoot), 'correct pre stateRoot')
Expand All @@ -81,7 +84,6 @@ module.exports = async function runBlockchainTest(options, testData, t) {
await cacheDB.close()
}

//console.log(testData)
const numBlocks = testData.blocks.length
let currentBlock = 0
let lastBlock = false
Expand All @@ -97,8 +99,8 @@ module.exports = async function runBlockchainTest(options, testData, t) {

try {
const block = new Block(Buffer.from(raw.rlp.slice(2), 'hex'), {
hardfork,
})
common
})

try {
await blockchain.putBlock(block)
Expand Down
Loading