Buffer to Uint8Array cleanup #2607

acolytec3 · 2023-03-29T01:09:29Z

Cleans up various buffer instantiations that were missed in previous Uint8Array pr.
Fixes devp2p logging bug
Addresses multiple places where Uint8Array values that are put in the database are transformed into JSON objects consisting of key/value pairs where the key is the array index and the value is that uint8 from the original uint8Array. This occurs because we have several places where the valueEncoding of our data being put in the DB is json which always maps to a JSON object (or nestred object) of utf-8 elements.
Adds a new PoW miner integration test to the client test suite

* V7 update to master 1 (#2593) * Added v7 release reference in main README table (#2562) * common: Schedule shanghai on goerli (#2563) * common: Schedule shanghai on goerli * update timestamp * util/tx: Shift ssz back to case dependency free ES2019 compatible version (#2564) * util/tx: Shift ssz back to case dependency free ES2019 compatible version * update package lock * update karma ecma version * VM: some optimization on the bnadd/bnmul precompiles to only copy over the necessary 128 bytes as input for the WASM call (#2568) * EVM: Memory Fix & Other Optimizations (#2570) * EVM: Rename evm debug logger to evm:evm (one for package, one for class), consistency, also, logger will otherwise be left out when run with evm:* * VM: Rename message checkpoint to state checkpoint in debug message (there is a dedicated message checkpoint msg along msg logging) * EVM: CALL/CREATE debug exit msg differentiation * EVM: avoid buffer copy in memory read (performance) * EVM: Rewrite runCall() checkpoint/revert conditional for readability/simplification * EVM: Added EIP check for transient storage checkpointing * EVM: Precompile Debug Logger Improvements (#2572) * EVM: Added general precompile debug logger setup, first ECRECOVER exemplary debug logging * EVM: Added remaining precompile debug loggers * EVM: added error cases to BLS precompile debug log * EVM: Added missing precompile return value debug logs * Small fixes * tx: ensure eip3860 txs can have more than max_initcode_size data if to field is non-empty (#2575) * EVM: Avoid memory.read() Memory Copy (#2573) * EVM: added avoidCopy parameter to memory.read() function, first test on CREATE opcode * EVM: Add direct memory read to all calling opcodes * EVM: Copy over memory on IDENTITY precompile * EVM: remove length checks and return buffer 0-filling in Memory.read() (memory is uncoditionally being extended properly anyhow) * Some optimizations * blockchain: fix merge->clique transition (#2571) * Client: ensure safe/finalized blocks are part of the canonical chain on forkchoiceUpdated (#2577) * client/engine: ensure finalized/safe blocks are in canonical chain * client: engine-api: fix finalized block check * client/tests: fix forkchoice updated test * client: add fcu tests to check if blocks are part of canonical chain * client/engine: ensure payload has a valid timestamp forkchoiceUpdated (#2579) * client/engine: ensure invalid blockhash response matches spec (#2583) * client/engine: delete invalid skeleton blocks (#2584) Co-authored-by: acolytec3 <17355484+acolytec3@users.noreply.github.com> * Setup to dev/test snapsync with sim architecture (#2574) * Setup to dev/test snapsync with sim architecture * modfiy single-run to setup a lodestar<>geth node to snapsync from * setup an ethereumjs inline client and get it to peer with geth * cleanup setup a bit * snapsync run spec * get the snap testdev sim working * finalize the test infra and update usage doc * enhance coverage * Use geth RPC to connect to ethJS * refac wait for snap sync completion --------- Co-authored-by: acolytec3 <17355484+acolytec3@users.noreply.github.com> * client: Add safe and finalized blockoptions to the chain (#2585) * client: Add safe and finalized blockoptions to the chain * fix tests * fix more tests * fix remaining * cleanup * enhance coverage * unset scheduled goerli timestamp based hfs colliding with test * Client: Small Debug Helpers and CLI Improvements (#2586) * Client: new constant MAX_TOLERATED_BLOCK_TIME for execution, added warning for slowly executed blocks * Client -> Execution: NumBlocksPerIteration (default: 50) as an option * Client: only restart RLPx server or log peer stats if max peers is set to be greater than 0 * Apply suggestions from code review Co-authored-by: acolytec3 <17355484+acolytec3@users.noreply.github.com> * Apply suggestions from code review --------- Co-authored-by: acolytec3 <17355484+acolytec3@users.noreply.github.com> * common: Schedule Shanghai on mainnet! (#2591) * common: Schedule Shanghai on mainnet! * clear hf timestamp for test * VM: Diff-based Touched Accounts Checkpointing (#2581) * VM: Switched to a more efficient diff-based way of touched account checkpointing * VM: move accessed storage inefficient checkpointing problem to berlin, haha * EVM: avoid memory copy in MLOAD opcode function * Remove console.log() in EVM * vmState: ensure touched accounts delete stack gets properly updated on commit * vm/eei: save touched height * vm/vmState: new possible fix for touched accounts * vm/vmState: another attempt to fix touched accounts journaling * vm: add journaling * Check correct journal height on revert --------- Co-authored-by: Jochem Brouwer <jochembrouwer96@gmail.com> Co-authored-by: acolytec3 <17355484+acolytec3@users.noreply.github.com> --------- Co-authored-by: Holger Drewes <Holger.Drewes@gmail.com> Co-authored-by: g11tech <gajinder@g11.in> Co-authored-by: Jochem Brouwer <jochembrouwer96@gmail.com> * First pass - most util changes * Fix most account stuff * Fix account test * Many byte fixes * util: fix constants tests * remaining fixes * Turn off ci jobs * monorepo: bigIntToUnpaddedBuffer -> bigIntToUnpaddedBytes * util: update description of bytes exporT * util: remove unused import * common: use bytesToHex helper instead of toString('hex') * trie: refactor non-test files to Uint8Array * util: add binary string utils * util: remove extra Uint8Array.from * util: remove arrToBufArray util * trie: adjust tests and fix outstanding issues * util: remove binarystring utils and add compareBytes and randomBytes util * common: refactor common with Uint8Array * util: accept 0x-prefixed and non-prefixed hex strings in toBytes * tx: refactor Buffer -> Uint8Array * tx: remove unused import * util: revert toBytes update * block: refactor Buffer -> uint8array * block: adjust import * trie: refactor remaining buffer instances * move devp2p to uint8Array * statemanager: refactor buffer -> uint8array * util: simplify zeros * util: add concatBytesUnsafe * ethash: partial migration * ethash: update examples * ethash: wip fixes * ethash: more WIP * ethash: ensure fnv input is read from mix, not mixView * blockchain: migrate to uint8array * ethash: renable all tests * ethash: fix bytesReverse * Fix miner tests * many hexToBytes moves * most of evm/vm moves * evm: Fix all tests * vm: more fixes * More fixes * vm: fix receipts encoding * vm: fix tester * client: refactor buffer to uint8array * client: additional uint8 adjustments * client: fix most tests * fix remaining client unit tests * reactivate most CI * client: fix les test/protocol * turn client CI on * util: fix name typo * lint * Fix withdrawals * remove buffarraytoarr * Remove bufArrtoArr references * Lint * fix examples * Fix difficulty test * lint * block: update randomBytes import * replace randombytes import * client: fix sim test util * vm: fix example * devp2p: update snappy typing and fix tests * Fix tests * Remove additional buffer references * rustbn fixes * add 0x prefix to precompile address * Remove `node-ip` dependency and buffer references in devp2p * Switch slice to subarray * evm: fix blake2f * Merge fixes * more merge commit fixes * more test fixes * Address all the feedback * fix dns test * Update packages/util/src/bytes.ts Co-authored-by: Jochem Brouwer <jochembrouwer96@gmail.com> * Fix return type for baToJSON * util: instantiate hexByByte array * Remove baToJson * rebase fixes * Fix event typing * Revert outdated initcode changes * lint --------- Co-authored-by: Holger Drewes <Holger.Drewes@gmail.com> Co-authored-by: g11tech <gajinder@g11.in> Co-authored-by: Jochem Brouwer <jochembrouwer96@gmail.com> Co-authored-by: Gabriel Rocheleau <contact@rockwaterweb.com>

codecov · 2023-03-29T01:14:54Z

Codecov Report

❗ No coverage uploaded for pull request base (develop-v7@54dc2e3). Click here to learn what that means.
The diff coverage is n/a.

Additional details and impacted files

Flag	Coverage Δ
block	`90.44% <0.00%> (?)`
blockchain	`90.54% <0.00%> (?)`
client	`87.10% <0.00%> (?)`
common	`95.76% <0.00%> (?)`
ethash	`∅ <0.00%> (?)`
evm	`79.48% <0.00%> (?)`
rlp	`∅ <0.00%> (?)`
statemanager	`89.58% <0.00%> (?)`
trie	`90.05% <0.00%> (?)`
tx	`94.45% <0.00%> (?)`
util	`83.54% <0.00%> (?)`
vm	`84.51% <0.00%> (?)`

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

jochem-brouwer · 2023-03-31T16:24:55Z

packages/client/test/miner/ethash.spec.ts

+  await client.start()
+  await new Promise((resolve) => {
+    client.config.logger.on('data', (data) => {
+      if (data.message.includes('Miner: Found PoW solution') === true && client.started) {


This test looks good, but the only problem I have here is that if this test fails (we broke the miner) this test will run endlessly and thus our CI will not finish. Maybe add a time limit of a minute or so, which rejects the promise if no solution is found?

Added a 60 second timeout to the test so the whole suite fails if it doesn't complete in 60 seconds (which I think should be sufficient).

jochem-brouwer · 2023-03-31T16:26:38Z

packages/client/test/miner/ethash.spec.ts

-  const restartedClient = await setupPowDevnet(minerAddress, false)
-  await restartClient(restartedClient, t)
+  await client.open()
+  await ((client.service('eth') as FullEthereumService).execution as any).stateDB!.open()


In general this is a great test too, so if we stop the client some internal variables are not cleaned up (so we cannot restart correctly). But this is a different issue, should be addressed though.

jochem-brouwer · 2023-03-31T16:28:23Z

packages/client/test/integration/pow.spec.ts

Shouldn't the test be:

Create inline client

Mine a PoW block

Stop the client

Restart the client

Mine a PoW block?

Right now it starts the client, does not mine, then stops and starts again and then mine a PoW block (this would reflect what we tested using the cli)

I think there must be some unclearness in how the function is designed. The setupPowDevnet function instantiates and starts the client. I've renamed the stopClient function to mineBlockAndStopClient to more clearly identify what happens in that helper. I set it up as an async function to make it easier to work with the event listener (which uses a callback rather than a promise interface and isn't naturally async). What actually happens is:

setupPowDevnet creates and starts the client

Test to verify the client is started

mineBlockAndStopClient sets a listener that waits for the first block to be mined and then passes its test and stops the client once it is mined.

If we try to stop and then restart the client. A second pow solution is never found so a next block is never mined.

Ok right this ethash problem is beyond the scope of this pr right?

packages/ethash/test/miner.spec.ts

g11tech · 2023-04-03T18:17:12Z

packages/evm/test/eips/eip-3860.spec.ts

@@ -135,7 +135,7 @@ tape('EIP 3860 tests', (t) => {
    const eei = await getEEI()
    const evm = await EVM.create({ common, eei, allowUnlimitedInitCodeSize: true })

-    const buffer = Buffer.allocUnsafe(1000000).fill(0x60)
+    const buffer = new Uint8Array(1000000).fill(0x60)


we might not need to fill with 0x00 although this is just test

Aren't we filling with 0x60 or is that equivalent to zero here?

g11tech

looks good, overall minor queries

packages/devp2p/src/rlpx/peer.ts

g11tech

👍

jochem-brouwer

LGTM

* `Buffer` to `Uint8Array` conversion (#2566) * V7 update to master 1 (#2593) * Added v7 release reference in main README table (#2562) * common: Schedule shanghai on goerli (#2563) * common: Schedule shanghai on goerli * update timestamp * util/tx: Shift ssz back to case dependency free ES2019 compatible version (#2564) * util/tx: Shift ssz back to case dependency free ES2019 compatible version * update package lock * update karma ecma version * VM: some optimization on the bnadd/bnmul precompiles to only copy over the necessary 128 bytes as input for the WASM call (#2568) * EVM: Memory Fix & Other Optimizations (#2570) * EVM: Rename evm debug logger to evm:evm (one for package, one for class), consistency, also, logger will otherwise be left out when run with evm:* * VM: Rename message checkpoint to state checkpoint in debug message (there is a dedicated message checkpoint msg along msg logging) * EVM: CALL/CREATE debug exit msg differentiation * EVM: avoid buffer copy in memory read (performance) * EVM: Rewrite runCall() checkpoint/revert conditional for readability/simplification * EVM: Added EIP check for transient storage checkpointing * EVM: Precompile Debug Logger Improvements (#2572) * EVM: Added general precompile debug logger setup, first ECRECOVER exemplary debug logging * EVM: Added remaining precompile debug loggers * EVM: added error cases to BLS precompile debug log * EVM: Added missing precompile return value debug logs * Small fixes * tx: ensure eip3860 txs can have more than max_initcode_size data if to field is non-empty (#2575) * EVM: Avoid memory.read() Memory Copy (#2573) * EVM: added avoidCopy parameter to memory.read() function, first test on CREATE opcode * EVM: Add direct memory read to all calling opcodes * EVM: Copy over memory on IDENTITY precompile * EVM: remove length checks and return buffer 0-filling in Memory.read() (memory is uncoditionally being extended properly anyhow) * Some optimizations * blockchain: fix merge->clique transition (#2571) * Client: ensure safe/finalized blocks are part of the canonical chain on forkchoiceUpdated (#2577) * client/engine: ensure finalized/safe blocks are in canonical chain * client: engine-api: fix finalized block check * client/tests: fix forkchoice updated test * client: add fcu tests to check if blocks are part of canonical chain * client/engine: ensure payload has a valid timestamp forkchoiceUpdated (#2579) * client/engine: ensure invalid blockhash response matches spec (#2583) * client/engine: delete invalid skeleton blocks (#2584) Co-authored-by: acolytec3 <17355484+acolytec3@users.noreply.github.com> * Setup to dev/test snapsync with sim architecture (#2574) * Setup to dev/test snapsync with sim architecture * modfiy single-run to setup a lodestar<>geth node to snapsync from * setup an ethereumjs inline client and get it to peer with geth * cleanup setup a bit * snapsync run spec * get the snap testdev sim working * finalize the test infra and update usage doc * enhance coverage * Use geth RPC to connect to ethJS * refac wait for snap sync completion --------- Co-authored-by: acolytec3 <17355484+acolytec3@users.noreply.github.com> * client: Add safe and finalized blockoptions to the chain (#2585) * client: Add safe and finalized blockoptions to the chain * fix tests * fix more tests * fix remaining * cleanup * enhance coverage * unset scheduled goerli timestamp based hfs colliding with test * Client: Small Debug Helpers and CLI Improvements (#2586) * Client: new constant MAX_TOLERATED_BLOCK_TIME for execution, added warning for slowly executed blocks * Client -> Execution: NumBlocksPerIteration (default: 50) as an option * Client: only restart RLPx server or log peer stats if max peers is set to be greater than 0 * Apply suggestions from code review Co-authored-by: acolytec3 <17355484+acolytec3@users.noreply.github.com> * Apply suggestions from code review --------- Co-authored-by: acolytec3 <17355484+acolytec3@users.noreply.github.com> * common: Schedule Shanghai on mainnet! (#2591) * common: Schedule Shanghai on mainnet! * clear hf timestamp for test * VM: Diff-based Touched Accounts Checkpointing (#2581) * VM: Switched to a more efficient diff-based way of touched account checkpointing * VM: move accessed storage inefficient checkpointing problem to berlin, haha * EVM: avoid memory copy in MLOAD opcode function * Remove console.log() in EVM * vmState: ensure touched accounts delete stack gets properly updated on commit * vm/eei: save touched height * vm/vmState: new possible fix for touched accounts * vm/vmState: another attempt to fix touched accounts journaling * vm: add journaling * Check correct journal height on revert --------- Co-authored-by: Jochem Brouwer <jochembrouwer96@gmail.com> Co-authored-by: acolytec3 <17355484+acolytec3@users.noreply.github.com> --------- Co-authored-by: Holger Drewes <Holger.Drewes@gmail.com> Co-authored-by: g11tech <gajinder@g11.in> Co-authored-by: Jochem Brouwer <jochembrouwer96@gmail.com> * First pass - most util changes * Fix most account stuff * Fix account test * Many byte fixes * util: fix constants tests * remaining fixes * Turn off ci jobs * monorepo: bigIntToUnpaddedBuffer -> bigIntToUnpaddedBytes * util: update description of bytes exporT * util: remove unused import * common: use bytesToHex helper instead of toString('hex') * trie: refactor non-test files to Uint8Array * util: add binary string utils * util: remove extra Uint8Array.from * util: remove arrToBufArray util * trie: adjust tests and fix outstanding issues * util: remove binarystring utils and add compareBytes and randomBytes util * common: refactor common with Uint8Array * util: accept 0x-prefixed and non-prefixed hex strings in toBytes * tx: refactor Buffer -> Uint8Array * tx: remove unused import * util: revert toBytes update * block: refactor Buffer -> uint8array * block: adjust import * trie: refactor remaining buffer instances * move devp2p to uint8Array * statemanager: refactor buffer -> uint8array * util: simplify zeros * util: add concatBytesUnsafe * ethash: partial migration * ethash: update examples * ethash: wip fixes * ethash: more WIP * ethash: ensure fnv input is read from mix, not mixView * blockchain: migrate to uint8array * ethash: renable all tests * ethash: fix bytesReverse * Fix miner tests * many hexToBytes moves * most of evm/vm moves * evm: Fix all tests * vm: more fixes * More fixes * vm: fix receipts encoding * vm: fix tester * client: refactor buffer to uint8array * client: additional uint8 adjustments * client: fix most tests * fix remaining client unit tests * reactivate most CI * client: fix les test/protocol * turn client CI on * util: fix name typo * lint * Fix withdrawals * remove buffarraytoarr * Remove bufArrtoArr references * Lint * fix examples * Fix difficulty test * lint * block: update randomBytes import * replace randombytes import * client: fix sim test util * vm: fix example * devp2p: update snappy typing and fix tests * Fix tests * Remove additional buffer references * rustbn fixes * add 0x prefix to precompile address * Remove `node-ip` dependency and buffer references in devp2p * Switch slice to subarray * evm: fix blake2f * Merge fixes * more merge commit fixes * more test fixes * Address all the feedback * fix dns test * Update packages/util/src/bytes.ts Co-authored-by: Jochem Brouwer <jochembrouwer96@gmail.com> * Fix return type for baToJSON * util: instantiate hexByByte array * Remove baToJson * rebase fixes * Fix event typing * Revert outdated initcode changes * lint --------- Co-authored-by: Holger Drewes <Holger.Drewes@gmail.com> Co-authored-by: g11tech <gajinder@g11.in> Co-authored-by: Jochem Brouwer <jochembrouwer96@gmail.com> Co-authored-by: Gabriel Rocheleau <contact@rockwaterweb.com> * Devp2p status fix * Remove buffer detritus * Switch db to view * buffer cleanup * Correctly parse heads from DB * Fix encodings * Cast db values to uint8array * Fix db bug in ethash * Remove unused peerId check * Add pow miner test * Finish test * Move pow test to integration tests * client: lint * rename test helper * Add timeout to PoW test * Fix test runner * remove console log --------- Co-authored-by: Holger Drewes <Holger.Drewes@gmail.com> Co-authored-by: g11tech <gajinder@g11.in> Co-authored-by: Jochem Brouwer <jochembrouwer96@gmail.com> Co-authored-by: Gabriel Rocheleau <contact@rockwaterweb.com>

acolytec3 and others added 5 commits March 27, 2023 13:41

Devp2p status fix

d9c020c

Remove buffer detritus

da78efe

Switch db to view

71e3a3a

buffer cleanup

f35554d

acolytec3 added package: devp2p package: client package: vm package: mpt labels Mar 29, 2023

acolytec3 marked this pull request as draft March 29, 2023 01:10

acolytec3 added 8 commits March 29, 2023 11:18

Correctly parse heads from DB

f16ebde

Fix encodings

20f41b1

Cast db values to uint8array

61894c6

Fix db bug in ethash

b43ab51

Remove unused peerId check

3e70fd7

Add pow miner test

047e916

Finish test

9bcd6dc

Move pow test to integration tests

a23e6fa

acolytec3 added package: blockchain package: tx package: ethash package: util package: evm and removed package: vm labels Mar 31, 2023

acolytec3 marked this pull request as ready for review March 31, 2023 16:09

acolytec3 requested a review from jochem-brouwer March 31, 2023 16:09

jochem-brouwer reviewed Mar 31, 2023

View reviewed changes

jochem-brouwer and others added 3 commits March 31, 2023 18:34

client: lint

20d8d8c

rename test helper

fdf8a79

Add timeout to PoW test

cd6f431

acolytec3 requested review from jochem-brouwer and g11tech March 31, 2023 18:12

g11tech reviewed Apr 3, 2023

View reviewed changes

packages/ethash/test/miner.spec.ts Show resolved Hide resolved

g11tech reviewed Apr 3, 2023

View reviewed changes

g11tech previously approved these changes Apr 3, 2023

View reviewed changes

holgerd77 force-pushed the develop-v7 branch from 6ea9dba to 54dc2e3 Compare April 4, 2023 06:53

Merge remote-tracking branch 'origin/develop-v7' into devp2p-fixes

9ec318b

acolytec3 dismissed g11tech’s stale review via 9ec318b April 4, 2023 14:01

Fix test runner

affa49b

g11tech reviewed Apr 4, 2023

View reviewed changes

packages/devp2p/src/rlpx/peer.ts Outdated Show resolved Hide resolved

remove console log

75ce24b

g11tech approved these changes Apr 4, 2023

View reviewed changes

jochem-brouwer approved these changes Apr 4, 2023

View reviewed changes

acolytec3 merged commit fd2e4ea into develop-v7 Apr 4, 2023

acolytec3 deleted the devp2p-fixes branch April 4, 2023 17:28

holgerd77 added PR state: needs review target: develop-v7 type: maintenance type: enhancement labels Apr 18, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Buffer to Uint8Array cleanup #2607

Buffer to Uint8Array cleanup #2607

acolytec3 commented Mar 29, 2023 •

edited

Loading

codecov bot commented Mar 29, 2023 •

edited

Loading

jochem-brouwer Mar 31, 2023

acolytec3 Mar 31, 2023

jochem-brouwer Mar 31, 2023

jochem-brouwer Mar 31, 2023

acolytec3 Mar 31, 2023

jochem-brouwer Mar 31, 2023

g11tech Apr 3, 2023

acolytec3 Apr 4, 2023

g11tech left a comment

g11tech left a comment

jochem-brouwer left a comment

Buffer to Uint8Array cleanup #2607

Buffer to Uint8Array cleanup #2607

Conversation

acolytec3 commented Mar 29, 2023 • edited Loading

codecov bot commented Mar 29, 2023 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

g11tech left a comment

Choose a reason for hiding this comment

g11tech left a comment

Choose a reason for hiding this comment

jochem-brouwer left a comment

Choose a reason for hiding this comment

acolytec3 commented Mar 29, 2023 •

edited

Loading

codecov bot commented Mar 29, 2023 •

edited

Loading