-
Notifications
You must be signed in to change notification settings - Fork 296
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
rpcserver: return rule error on rejected raw tx #808
Conversation
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 will need some changes.
First off, the comment above is now wrong because it talks about how it is not returning a deserialization error, yet now it does.
Second, this is not the behavior we really want, even if this is what bitcoind does (or did at time of comment). What we do want is to use an error code that specifies that the input was rejected due to either consensus or policy rules. Unfortunately, no such error code exists at this time. I think we should cross reference bitcoind to see if they have added anything to handle this and if so use it, otherwise we can add our own error code.
Finally, the first line of the commit message makes no sense. This change does not touch the wire package at all, and instead only affects the JSON-RPC server.
ca5b125
to
f0c21a0
Compare
f0c21a0
to
5f04432
Compare
I've cross referenced with bitcoind and it's also using the deserialization error type for the rule error case. Now using |
rpcserver.go
Outdated
} else { | ||
rpcsLog.Errorf("Failed to process transaction %v: %v", | ||
tx.Hash(), err) | ||
rpcsLog.Debugf(err.Error(), "") |
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.
All of these should use "%v" or "%s" as the format string, not the error string.
5f04432
to
8bbe65e
Compare
@jrick updated the PR per your review. |
8bbe65e
to
4438181
Compare
Closes #733