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

fix vm blockchain.delBlock parameter #63

Merged
merged 2 commits into from
May 9, 2016
Merged

fix vm blockchain.delBlock parameter #63

merged 2 commits into from
May 9, 2016

Conversation

kumavis
Copy link
Member

@kumavis kumavis commented Apr 30, 2016

No description provided.

@@ -52,7 +52,8 @@ module.exports = function (blockchain, cb) {
}, function (err, results) {
if (err) {
// remove invalid block
self.stateManager.blockchain.delBlock(block, cb)
console.error(err)
Copy link
Member

Choose a reason for hiding this comment

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

Please remove the log

@kumavis
Copy link
Member Author

kumavis commented Apr 30, 2016

We need an optional logging mechanism -- the error here could be anything but we discard it and just attempt to delete the block. In my case the error was an important piece of evidence in tracking down a bug, but would have lost it..

any idea on optional logging mechanisms? just check self.debug ?

@axic
Copy link
Member

axic commented Apr 30, 2016

Having a logging interface would be nice, but console.log probably isn't the best way to go in a library.

For starters it could be just an optional log in self, which is ignored by default, unless the user sets it to a method?

@wanderer
Copy link
Member

wanderer commented May 1, 2016

recap on a conversation with @kumavis ; I think he can get all the information need from vm.on('block', cb) instead of built in loggin.

@kumavis kumavis self-assigned this May 3, 2016
@wanderer
Copy link
Member

wanderer commented May 9, 2016

@kumavis will merge! just take out that console.log :)

@kumavis
Copy link
Member Author

kumavis commented May 9, 2016

@wanderer removed

@wanderer wanderer merged commit 9c8ce3e into master May 9, 2016
@kumavis kumavis deleted the kumavis-patch-2 branch May 11, 2016 00:37
evertonfraga pushed a commit to evertonfraga/ethereumjs-vm that referenced this pull request Feb 11, 2020
holgerd77 pushed a commit that referenced this pull request Dec 1, 2020
holgerd77 added a commit that referenced this pull request Dec 14, 2020
holgerd77 added a commit that referenced this pull request May 26, 2023
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.

4 participants