rpcclient: add bitcoind version dependent error matching #2405
Conversation
Pull Request Test Coverage Report for Build 17062080212Details
💛 - Coveralls |
|
@ziggie1984 maybe you could take a look? |
| // The error message was changed in | ||
| // https://github.com/bitcoin/bitcoin/pull/33050 which will be included | ||
| // in bitcoind v30.0 and beyond. | ||
| "mempool script verify flag failed": ErrNonMandatoryScriptVerifyFlag, |
There was a problem hiding this comment.
can you also add: block-script-verify-flag-failed see https://github.com/bitcoin/bitcoin/blob/master/doc/release-notes-33183.md ?
| // version dependent (e.g. versions up to v29 return the error as specified in | ||
| // `Error()` above, while versions v30 and beyond return the error as mapped | ||
| // here. | ||
| var BitcoindErrMap = map[string]error{ |
There was a problem hiding this comment.
Why don't we just introduce 2 new errors instead of mapping it to the old one ?
There was a problem hiding this comment.
Because the errors were simply renamed, I wanted to keep the same semantic meaning (e.g. the same error constant) in case we matched on them anywhere. We currently don't, so we could've added new errors. But I still find it slightly better in terms of understanding what's going on. Added some additional context to the comment of the new map, why we use it and when we should create new constants.
I hope that makes sense to you?
Fixes btcsuite#2404. If different versions of bitcoind return different error strings, we need a way to match those as well.
1435f8c to
ffcda0f
Compare
|
@Roasbeef requesting merge override. |
Fixes #2404.
If different versions of bitcoind return different error strings, we need a way to match those as well.