-
Notifications
You must be signed in to change notification settings - Fork 782
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
Buffer to Uint8Array cleanup #2607
Merged
Merged
Changes from 1 commit
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
6ea9dba
`Buffer` to `Uint8Array` conversion (#2566)
acolytec3 d9c020c
Devp2p status fix
acolytec3 da78efe
Remove buffer detritus
acolytec3 71e3a3a
Switch db to view
acolytec3 f35554d
buffer cleanup
acolytec3 f16ebde
Correctly parse heads from DB
acolytec3 20f41b1
Fix encodings
acolytec3 61894c6
Cast db values to uint8array
acolytec3 b43ab51
Fix db bug in ethash
acolytec3 3e70fd7
Remove unused peerId check
acolytec3 047e916
Add pow miner test
acolytec3 9bcd6dc
Finish test
acolytec3 a23e6fa
Move pow test to integration tests
acolytec3 20d8d8c
client: lint
jochem-brouwer fdf8a79
rename test helper
acolytec3 cd6f431
Add timeout to PoW test
acolytec3 9ec318b
Merge remote-tracking branch 'origin/develop-v7' into devp2p-fixes
acolytec3 affa49b
Fix test runner
acolytec3 75ce24b
remove console log
acolytec3 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,6 +9,7 @@ import { Config } from '../../lib' | |
import { createInlineClient } from '../sim/simutils' | ||
|
||
import type { EthereumClient } from '../../lib' | ||
import type { FullEthereumService } from '../../lib/service' | ||
|
||
const pk = hexToBytes('95a602ff1ae30a2243f400dcf002561b9743b2ae9827b1008e3714a5cc1c0cfe') | ||
const minerAddress = Address.fromPrivateKey(pk) | ||
|
@@ -94,7 +95,6 @@ const stopClient = async (client: EthereumClient, t: tape.Test) => { | |
} | ||
|
||
const restartClient = async (client: EthereumClient, t: tape.Test) => { | ||
await client.start() | ||
await new Promise((resolve) => { | ||
client.config.logger.on('data', (data) => { | ||
if (data.message.includes('Miner: Found PoW solution') === true && client.started) { | ||
|
@@ -110,7 +110,10 @@ tape('start client', async (t) => { | |
const client = await setupPowDevnet(minerAddress, true) | ||
t.ok(client.started, 'client started successfully') | ||
await stopClient(client, t) | ||
const restartedClient = await setupPowDevnet(minerAddress, false) | ||
await restartClient(restartedClient, t) | ||
await client.open() | ||
await ((client.service('eth') as FullEthereumService).execution as any).stateDB!.open() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
await client.chain.blockchain.db.open() | ||
await client.start() | ||
await restartClient(client, t) | ||
t.end() | ||
}) |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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).