-
Notifications
You must be signed in to change notification settings - Fork 200
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
Sanitize logs #495
Merged
Merged
Sanitize logs #495
Conversation
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
timmoreton
approved these changes
Sep 25, 2019
kevjue
added a commit
that referenced
this pull request
Sep 25, 2019
* Check message address against signature (#477) * Check signing validator's address matches msg address * Add comments about use of sig data in tests * Try fix Circle build test failures * Try fix Circle build test failures, take 2 * tx price heap fix (#471) * contract_comm/currency/currency.go * fixed the txn price-sorted min-heap * merge master (#490) * Add precompiles to access validator set (#441) * set max gas to double of the charged gas for the 'intrinsic' smart contract calls (#472) * set max gas to double of the charged gas for the 'intrinsic' evm operations * addressed PR comments * addressed pr comment * Adds Prepared Certificates to ensure Istanbul liveness (#366) * Check message address against signature (#477) * Check signing validator's address matches msg address * Add comments about use of sig data in tests * Try fix Circle build test failures * Try fix Circle build test failures, take 2 * added new option --use-in-memory-discovery-table (#479) * added new option --use-in-memory-discovery-table * merge master (#489) * Adds Prepared Certificates to ensure Istanbul liveness (#366) * Check message address against signature (#477) * Check signing validator's address matches msg address * Add comments about use of sig data in tests * Try fix Circle build test failures * Try fix Circle build test failures, take 2 * Allow v4/v5 on a bootnode simultaneously, allow mobile to use discv5 (#454) * changes for isolating celo networks * changes to get unit tests working * changes to add salt in the discovery packets * removed checking for the ip address when handling a reply * added ping-ip-from-packet option to bootnode * for handling expected replies, don't filter on expected sender ip address if --pingIPFromPacket is used * Add v4 flag * Add unhandled and quicken docker builds * Add salt & logs * Add v4 flag * Add PeerDiscovery to mobile node config * Remove logs * Remove hardcoded bootnodes * Add salt & turn on discv5 * Delete hardcoded eth bootnodes * Make Discoveryv5 configurable * Lint * Add comment to bootnode v4/v5 handling * Change PeerDiscovery -> NoDiscovery * Remove mobile geth no discovery * Reduce istanbul default timeout, cap exp backoff (#475) * Reduce istanbul default timeout, cap exp backoff * Ensure round 0 timeout factors in block period * Sanitize logs (#495) * Change registry lookup and infrastructure lookup error logs to debug level * Sanitize logs regarding registry deployment * Change empty abi logging and comment * Lower log level from error to warning for potentially outdated istanbul messages * Change back to an error message * Add input length checks for precompiled contracts (#476) * add input length checks * check exact input length. add a new error for input length. check input in a few more places * add tests that verify the input-length checks for contracts that don't require an evm instance * fix formatting * add comments to explain input length checks * run the formatter * e2e transfer test was failing because it passes in a transaction options object, making the input larger than 96 bytes * e2e tests have revealed that our precompiled contracts need to be tolerant of inputs that are longer than the bytes that are actually read
kevjue
pushed a commit
that referenced
this pull request
Sep 26, 2019
* Log on ValidatorElections * merge master (#496) * Check message address against signature (#477) * Check signing validator's address matches msg address * Add comments about use of sig data in tests * Try fix Circle build test failures * Try fix Circle build test failures, take 2 * tx price heap fix (#471) * contract_comm/currency/currency.go * fixed the txn price-sorted min-heap * merge master (#490) * Add precompiles to access validator set (#441) * set max gas to double of the charged gas for the 'intrinsic' smart contract calls (#472) * set max gas to double of the charged gas for the 'intrinsic' evm operations * addressed PR comments * addressed pr comment * Adds Prepared Certificates to ensure Istanbul liveness (#366) * Check message address against signature (#477) * Check signing validator's address matches msg address * Add comments about use of sig data in tests * Try fix Circle build test failures * Try fix Circle build test failures, take 2 * added new option --use-in-memory-discovery-table (#479) * added new option --use-in-memory-discovery-table * merge master (#489) * Adds Prepared Certificates to ensure Istanbul liveness (#366) * Check message address against signature (#477) * Check signing validator's address matches msg address * Add comments about use of sig data in tests * Try fix Circle build test failures * Try fix Circle build test failures, take 2 * Allow v4/v5 on a bootnode simultaneously, allow mobile to use discv5 (#454) * changes for isolating celo networks * changes to get unit tests working * changes to add salt in the discovery packets * removed checking for the ip address when handling a reply * added ping-ip-from-packet option to bootnode * for handling expected replies, don't filter on expected sender ip address if --pingIPFromPacket is used * Add v4 flag * Add unhandled and quicken docker builds * Add salt & logs * Add v4 flag * Add PeerDiscovery to mobile node config * Remove logs * Remove hardcoded bootnodes * Add salt & turn on discv5 * Delete hardcoded eth bootnodes * Make Discoveryv5 configurable * Lint * Add comment to bootnode v4/v5 handling * Change PeerDiscovery -> NoDiscovery * Remove mobile geth no discovery * Reduce istanbul default timeout, cap exp backoff (#475) * Reduce istanbul default timeout, cap exp backoff * Ensure round 0 timeout factors in block period * Sanitize logs (#495) * Change registry lookup and infrastructure lookup error logs to debug level * Sanitize logs regarding registry deployment * Change empty abi logging and comment * Lower log level from error to warning for potentially outdated istanbul messages * Change back to an error message * Add input length checks for precompiled contracts (#476) * add input length checks * check exact input length. add a new error for input length. check input in a few more places * add tests that verify the input-length checks for contracts that don't require an evm instance * fix formatting * add comments to explain input length checks * run the formatter * e2e transfer test was failing because it passes in a transaction options object, making the input larger than 96 bytes * e2e tests have revealed that our precompiled contracts need to be tolerant of inputs that are longer than the bytes that are actually read
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
This decreases the logging intensity for a few expected situations. The first is when geth tries to get the the registry contract, but the contract is not yet deployed. This will output a lot of
error
level logs when a node begins syncing-- this is because geth will try to get the registry contract when processing a block, and there will always be a series of blocks that were mined before the registry contract was deployed. I decreased these logs the todebug
level.There were 2 other
error
level logs to do with Istanbul that I changed fromerror
->warning
. The first is when an error occurs when upserting a validator enode entry (which can happen when an old announce message is received-- there was a comment on discord about this). The second is when handling an istanbul message that was sent from a backlog event (which can also happen from an old announce message).Logs are now pretty clear & easy to read on log level 3
Tested
Created my own testnet with the new logging
Other changes
n/a
Related issues
Backwards compatibility
The changes are backwards compatible