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

Develop rebased on 3e4e7be #1928

Closed
wants to merge 47 commits into from
Closed

Conversation

g11tech
Copy link
Contributor

@g11tech g11tech commented Jun 2, 2022

rebased on

commit 3e4e7bed681bfa6d1c95abbcc28f6508e828339d (HEAD -> master, origin/master, origin/HEAD)
Author: Holger Drewes <Holger.Drewes@gmail.com>
Date:   Thu Jun 2 22:14:19 2022 +0200

    Blockchain: Clique-recently-signed False Positive Fix (#1931)
    

@holgerd77
Copy link
Member

holgerd77 commented Jun 2, 2022

Oh, that's so so great, thanks a lot Gajinder!!! ❤️ 🙏 🚀

To prepare for the subsequent process: so there is this last master commit from the v5 releases PR still missing, otherwise this will be the very last time that we have rebased anything here (so: at least in this develop towards master directly, might be some develop-feature-branch -> develop rebases still be necessary) 🎉.

I guess (hope 😬) this will be easily integratable here (basically by an additional rebase towards this last commit, should not be to "conflicty" since only/mostly doc files and package.json files are touched?).

So: I will basically merge "right now" (or in the next 30 minutes or so).

Side note: I guess we might be early here then - depending on how the rebase is going, will likely be a tricky last one - and eventually directly switch onto the newly rebased develop branch from here tomorrow (so being early according to our planning issue actually 😋: #1914)

@holgerd77
Copy link
Member

Update: merged in the release PR (so: #1927)

@holgerd77
Copy link
Member

Another update: if you re-rebase 😋: wait a minute, I guess I would have needed to submit an updated package-lock.json at the end of the version number updates of the release PR. Will do this in a small separate PR (which I would also admin-merge). Will give an update here.

@holgerd77
Copy link
Member

I've got a bug in the client which blocks me from doing the release round: #1930

If you have some capacity right now (or relatively soon) and could re-prioritize and have a look that would be appreciated! 🙂

@g11tech g11tech force-pushed the develop-rebased-on-30ca12e6 branch from f781f06 to a81f099 Compare June 2, 2022 10:17
@holgerd77
Copy link
Member

(can be fixed just by a normal PR towards master)

@g11tech g11tech changed the title Develop rebased on 30ca12e6 Develop rebased on 52c6d52 Jun 2, 2022
@g11tech g11tech force-pushed the develop-rebased-on-30ca12e6 branch from a81f099 to 0cf5815 Compare June 2, 2022 10:21
@g11tech g11tech changed the title Develop rebased on 52c6d52 Develop rebased on 5e266fffa Jun 2, 2022
@holgerd77
Copy link
Member

I now merged in #1924. This would need to be cherry-picked on top here (git cherry-pick d62b7f4) once the PR is generally ready.

(@g11tech let me know if these kind of merges pose any problems and you would prefer an alternative procedure to go forward here right now)

@g11tech
Copy link
Contributor Author

g11tech commented Jun 3, 2022

I now merged in #1924. This would need to be cherry-picked on top here (git cherry-pick d62b7f4) once the PR is generally ready.

(@g11tech let me know if these kind of merges pose any problems and you would prefer an alternative procedure to go forward here right now)

cherry picking sounds good @holgerd77 👍

@codecov
Copy link

codecov bot commented Jun 3, 2022

Codecov Report

Merging #1928 (f15506c) into master (3e4e7be) will increase coverage by 1.66%.
The diff coverage is 86.13%.

❗ Current head f15506c differs from pull request most recent head 5d5a841. Consider uploading reports for the commit 5d5a841 to get more accurate results

Impacted file tree graph

Flag Coverage Δ
block 86.15% <89.92%> (+0.57%) ⬆️
blockchain 81.72% <78.85%> (-1.88%) ⬇️
client ?
common 95.01% <95.00%> (+0.82%) ⬆️
devp2p 82.64% <77.21%> (-0.05%) ⬇️
ethash 90.81% <100.00%> (+0.04%) ⬆️
rlp 90.55% <ø> (ø)
statemanager 84.55% <73.52%> (?)
trie 80.19% <ø> (+0.17%) ⬆️
tx 92.18% <ø> (+3.90%) ⬆️
util 87.32% <ø> (-5.32%) ⬇️
vm 80.26% <ø> (-0.76%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

@holgerd77
Copy link
Member

holgerd77 commented Jun 3, 2022

I've now also merged in #1867, would also need to be cherry-picked (git cherry-pick a7b1648bc40ee6bd2f33732017530d27c14be27e).

This is the last PR I would merge, so we are now on both the final (for the rebase) develop AND master state. So once this is working here we can take this over as the new (and: last) develop state! 💯 🎉

Will now do the last round of v5 releases and also branch off the v5-maintenance branch.

jochem-brouwer and others added 13 commits June 3, 2022 17:34
* vm: add codeHash db prefix to stateManager

* vm: tests: make tests use StateManager instead of trie at utilization
vm: tests: remove now passing test from skipTests

* vm: fix tests / fix runBlock/runBlockchain tests

* vm: tests: fix cleaning up untouched accounts in tests

* vm -> StateManager: added comment on CODEHASH_PREFIX

Co-authored-by: holgerd77 <Holger.Drewes@gmail.com>
Co-authored-by: steveluscher <github@steveluscher.com>
…orks to VM (#1649)

* common: remove onlySupported and onlyActive hf options

* vm: move supportedHardforks option from common

* common: remove supportedHardforks option

* common: update README for supportedHardforks removal

* vm: update README for supportedHardforks addition

* common: cleanup in src/index.ts

* vm: refactor supportedHardforks per PR feedback

* common: remove _chooseHardfork method per PR feedback

* vm: updated README per PR feedback

* vm: supportedHardforks local variable, switch to enum
* common: delete deprecated hardforkBlock method

* common: adapt tests to use hardforkBlockBN

* common: delete deprecated nextHardforkBlock method

* common/tests: rename nextHardforkBlock() calls
to nextHardforkBlockBN()

* common: adapt tests to use BN.eqn(number)

* common: delete deprecated networkId() method

* common: delete tests for networkId()

* common: delete deprecated chainId() method

* common: delete tests for .chainId()

* delete deprecated forCustomChain meethod

* common: delete forCustomChain() test

* blockchain: switch forCustomChain to Common.custom

* client: switch fromCustomChain to Common.custom

* tx: switch forCustomChain to Common.custom

* VM: switch forCustomChain to Common.custom

* common: internalize _getInitializedChains() method

* common: delete deprecaed chains/index.ts file

* client: switch to Common._getInitializedChains

* common: remove genesisStates/index.ts

* common: remove test for genesisStates
* common: rename nextHardforkBlockBN()
to nextHardforkBlock()

* common/tests: rename nextHardforkBlockBN()
to nextHardforkBlock()

* client: rename common.nextHardforkBlockBN()
to common.nextHardforkBlock()

* devp2p: rename common.nextHardforkBlockBN()
to common.nextHardforkBlock()

* common: rename hardforkBlockBN()
to hardforkBlock()

* common/tests: rename common.hardforkBlockBN()
to common.hardforkBlock()

* block: rename common.hardforkBlockBN()
to common.hardforkBlock()

* client: rename common.hardforkBlockBN()
to common.hardforkBlock()

* blockchain: rename common.hardforkBlockBN()
to common.hardforkBlock()

* devp2p: rename common.hardforkBlockBN()
to common.hardforkBlock()

* vm: rename common.hardforkBlockBN()
to common.hardforkBlock()

* common: rename chainIdBN()
to chainId()

* common/tests: rename common.chainIdBN()
to common.chainId()

* client: rename common.chainIdBN()
to common.chainId()

* blockchain: rename common.chainIdBN()
to common.chainId()

* devp2p: rename common.chainIdBN()
to common.chainId()

* tx: rename common.chainIdBN()
to common.chainId()

* vm: rename common.chainIdBN()
to common.chainId()

* common: rename networkIdBN() to networkId()

* common/test: rename common.networkIdBN()
to common.networkId()

* client: rename common.networkIdBN()
to common.networkId()

* block: rename common.networkIdBN()
to common.networkId()
* Replace BN.js with bigints in VM

* Fix byteLength computation in EXP handler

* Fix toTwos helper

* Compute TWO_POWE256 using math instead of hex

* Update packages/util/src/constants.ts

Co-authored-by: Ryan Ghods <ryan@ryanio.com>

* Remove unused variable

* Fix exponent byte length calc

* Fix exp overflow

* Fix precompile bigint conversions

* Fix more bigint conversions

* Fix EXP opcode handler

* Fix logic bug in signextend

* vm/gas: fix EXTCODECOPY gas

* vm/gas: fix sha256 gas

* vm/gas: sha256 const -> let

* vm: lint

* vm/logic: fix sdiv/smod and fromTwos/toTwos

* vm/logic: fix SIGNEXTEND

* vm/logic: fix CALLDATALOAD padding

* vm/logic: remove weird comment

* Fix SAR opcode handler

* Use bufferToBigInt in Push opcode handler

* use bufferToBigInt everywhere

* Fix missing bufferToBigInt import

* Check for edge case in expmod

* Ignore prettier

* Update browser tsconfig to es2020 lib

* Remove dup ES2020 targets

* attempt to dedupe "target" and "lib" tsconfig values

* Update karma config to target es2020 in parser

* Various test fixes

* Lint and BN fixes

* Add bigint helpers to util

* lint fixes

* various bigint helper additions

* Lint fixes

* Fix bnToBigInt

* Lint/test fixes

* Switch Xn to BigInt(X)

* lint

* lint

* More Xn to BigInt(X) moves

Co-authored-by: Ryan Ghods <ryan@ryanio.com>
Co-authored-by: Jochem Brouwer <jochem.brouwer@nslogin.nl>
Co-authored-by: Jochem Brouwer <jochembrouwer96@gmail.com>
* Tx: Remove baseTransaction.transactionType

* tx: remove get senderR()

* tx: remove test for senderR()

* tx: remove senderS()

* tx: remove test for senderS()

* tx: remove yParity / replace with v

* tx: remove test for yParity()

* Tx: remove fromRlpSerializedTx()

* Tx: remove test for fromRlpSerializedTx()

* Tx: remove _unsignedTxImplementsEIP155

* Tx: Remove _signedTxImplementsEIP155()

* Tx: Remove TransactionFactory.getTransactionClass

* Tx: Remove test for .getTransactionClass()
* common: set default hardfork to london

* vm: set default hardfork to london

* vm: update tests after default hardfork change

* vm: update default hardfork in tester config

* block: update default hardfork and fix tests

* blockchain: fix tests for london as default hardfork

* tx: set default hardfork to london

* block: fix for uncles at hardfork transition test

* blockchain: fix for hardfork in reorg test

* ethash: fix ethash tests for default hardfork change

* tx: remove README section about hardforks

* client: fix client tests for default hardfork change

* block: rename testdata files for pre-london

* blockchain: rename testdata files for pre-london

* vm: fix examples for London hardfork change

Co-authored-by: Emerson Macro <emersonmacro@gmail.com>
* common: rename hardforkIsActiveOnChain to isIncludedHardfork

* common: remove activeHardfork and activeHardforks

* common: adapt _getHardfork, remove isIncludedHardfork

* common: type fixes in forkHash
* add modifyAccountFields method and tests
* use new method in eei
* update usage in various tests
acolytec3 and others added 9 commits June 3, 2022 17:35
* Common: Return BigInt for param() functionality
Fixes #1846

* Change param getters to throw on non-existent value

* Fixes

* switch return to bigint or undefined

* Switch param getters to return undefined and fix common tests

* tx: changes to accomodate param getters

* More changes

* More adjustments

* Fix NaN bug in opcode fee builder

* Hack to fix EIP-2537 edge case

* client: fix test syntax

* lint

* Return 0 when param doesn't exist

* Spelling, dedup param usage

* Update `paramByEip` to allow undefined

* Hardcode EIP-2537 gas discount array

* uncomment test
* block: update to rlp v3

* blockchain: update to rlp v3

* client: update to rlp v3

* devp2p: update to rlp v3

* ethash: fix test to pass buffer

* trie: update to rlp v3

* tx: update to rlp v3

* util: update to rlp v3

* vm: update to rlp v3

* lint, etc. fixes and update package-lock

* address review comments
* blockchain: remove 'void' from iterator() return

* vm: remove void from vm.runBlockchain() return types

* client: adapt VMExecution to stricter blockchain.iterator() call

* vm: reintroduce deleted tests for blockchain.getIteratorHead()

* vm: correct method name in test
* VM -> EVM/VM Refactor: new EVMOpts EVM options dict, move common and supported HFs to EVM

* VM -> EVM/VM Refactor: standalone DEBUG property in EVM and Interpreter

* VM -> EVM/VM Refactor: standalone EVM stateManager option and property

* VM -> EVM/VM Refactor: moved runCall()/runCode() to EVM, deleted dedicated files, top-level EVM instantiation in VM

* VM -> EVM/VM Refactor: made EVM AsyncEventEmitter, moved beforeMessage, afterMessage, newContract, step events to EVM

* VM -> EVM/VM Refactor: moved allowUnlimitedContractSize option to EVM

* VM -> EVM/VM Refactor: moved opcode and gas handler functionality and options to EVM, removed EVM/Precompiles VM dependency

* Client: fix cliqueActiveSigners method assignments

* VM: refactor TxContext to an interface

* VM: improve Message class types and defaults handling

* VM: refactor runCall and executeMessage into unified runCall

* VM: fix gas refund reset to 0 after tx finishes and fix parenthesis in runBlock receipt ternary

* VM: reimplement transient storage clear method

* VM: use VM async create method instead of constructor

* VM: unify interpreter and evm DEBUG property

Co-authored-by: Holger Drewes <Holger.Drewes@gmail.com>
* Add typing to hardforkforForkhash

* Type initializedChains

* Union type for consensus algorithm

* Add check for invalid chain ID

* Add cliqueOpts type

* block: cast to cliqueOpts

* client: cast to cliqueOpts in miner

* Fix tests

* Add back chainsType import

* Add back missing types

* Address comments

* lint

* Address feedback
* common: create new common.ts

* common: finish reorganize/rename in common

* Address comments
* tx: edit legacyTransaction to throw when hash() called on unsigned tx

* tx: adapt legacy.spec test to throw on unsigned tx.hash()

* tx: edit tests which call hash() on unsigned tx
@g11tech g11tech force-pushed the develop-rebased-on-30ca12e6 branch from 208f232 to dc69483 Compare June 3, 2022 12:06
ryanio and others added 2 commits June 3, 2022 18:31
* simplify setting blockchan genesis parameters for future extensibility, make timestamp optional instead of `| null`

* move EOF one folder up, export default rather than wildcard import

* remove `assert` usage from vm/precompiles, devp2p

* eof: reinstate individual imports

* make genesisBlock a property and add a separate createGenesisBlock method

* fix mock, remove commented out code (double checked, geth does not return the eth protocol versions in admin_nodeInfo, so this is safely removable)

* remove old chain/hardfork check

* simplify errors, use increment operator, use for..of rather than forEach
* vm: update eip 3074

* vm: eip3074 test fixes start

* vm: fix eip3074 tests

* vm: expand eip3074 tests

* vm: add eip 3074 tests

* vm: address 128n -> BigInt(128)
@g11tech g11tech force-pushed the develop-rebased-on-30ca12e6 branch from dc69483 to e328bd5 Compare June 3, 2022 13:03
@g11tech g11tech changed the title Develop rebased on 5e266fffa Develop rebased on 3e4e7be Jun 3, 2022
@holgerd77
Copy link
Member

Oh, that already looks pretty good! 🙂

@holgerd77 holgerd77 mentioned this pull request Jun 7, 2022
}
}

let chainIdBN
Copy link
Member

Choose a reason for hiding this comment

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

This has been fixed in the "wrong direction", so the removed code is actually the correct one, see #1905 for context and discussion.

So fix should be just to replace v.ltn(37) && !v.eqn(27) && !v.eqn(28) with the normal BigInt-operator comparisons and literally nothing else.

(let me know if I overlook something)

Copy link
Member

Choose a reason for hiding this comment

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

Update: ah, just seeing that Andrew addressed this in a follow-up commit, so not an issue.

head: new BN(raw[0]),
tail: new BN(raw[1]),
head: bufferToBigInt(raw[0]),
tail: bufferToBigInt(raw[1]),
Copy link
Member

Choose a reason for hiding this comment

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

Ah, a lot of BigInt-related BeaconSync and Skeleton changes, makes sense! 🙂

default:
throw new Error(`tx of type ${tx.type} unknown`)
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Out of scope of this PR definitely but just for some thought: I wonder if such kind of a helper function would rather be some addition for the tx library itself? 🤔

}
}

let chainIdBN
Copy link
Member

Choose a reason for hiding this comment

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

Update: ah, just seeing that Andrew addressed this in a follow-up commit, so not an issue.

default:
throw new Error(`consensus algorithm ${this._common.consensusAlgorithm()} not supported`)
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Whew, that's a hard one: a whole new API method introduced in a rebase-fix-PR. 😋 😬

Are we confident enough that we want to expose? Or would we rather want to keep this private (or rather: protected) for now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yea it was tricky one to find! well will move to protected 👍

@g11tech g11tech force-pushed the develop-rebased-on-30ca12e6 branch 2 times, most recently from 617db09 to 2df3ff4 Compare June 7, 2022 19:08
 * fix legacyTransaction rebase

 * state manager build fx

 * rebase fixes

 * pick the packages/blockchain/src/index.js from develop

 * some more bn related fixes

 * update the fix corresponding to #1931

 * client fixes

 * Remove bn ops

 * fix vm test util

 * fix memory write extend

 * fix lint

 * update blockchain runner from develop

 * Add v check back for legacy Txs

 * Fix miner tests

 * resolve skeleton.getBlock errors because of mismatching config w.r.t. the one with which blocks were created

 * proper undefined comparision for bigint

 * provide genesis hash to skelteons block 1

 * switch consensus algo on hardfork consensus change

 * base fee proper fix

 * fix txpool specs

 * fix eth protocol spec

 * pending block spec fix

 * fix miner/miner spec

 * fix send raw transaction spec

 * reset the miner spec changes
@g11tech g11tech force-pushed the develop-rebased-on-30ca12e6 branch from 2df3ff4 to 5d5a841 Compare June 7, 2022 19:21
@g11tech
Copy link
Contributor Author

g11tech commented Jun 7, 2022

@holgerd77 @acolytec3 i think all set for promoting as master, though some fixes might followup as I try it out on testnets. but hopefully nothing major.

@holgerd77 holgerd77 marked this pull request as ready for review June 8, 2022 07:49
@holgerd77
Copy link
Member

Branch has been applied to master via #1943, will close.

@holgerd77 holgerd77 closed this Jun 8, 2022
@holgerd77 holgerd77 deleted the develop-rebased-on-30ca12e6 branch March 2, 2023 09:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.