-
Notifications
You must be signed in to change notification settings - Fork 590
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
chain: add TestMempoolAccept
to chain interface
#899
Conversation
7312742
to
36e41df
Compare
wallet/wallet.go
Outdated
@@ -4131,7 +4131,7 @@ func MapBroadcastBackendError(err error) error { | |||
// | |||
// https://github.com/bitcoin/bitcoin/blob/9bf5768dd628b3a7c30dd42b5ed477a92c4d3540/src/node/transaction.cpp#L49 | |||
// https://github.com/bitcoin/bitcoin/blob/0.20/src/validation.cpp#L642 | |||
case match(err, "missing inputs") || | |||
case match(err, "missing-inputs") || |
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.
😣
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.
Wondering what we can do in the future to catch stuff like this...
Ofc it's the case that string error matching is fragile to being with, wish we had some concrete error codes here.
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.
Or should we be looking at one of these sources instead?
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.
Also fwiw, this gives a 404 now:
https://github.com/bitcoin/bitcoin/blob/0.20/src/validation.cpp#L642
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.
These two are my primary source when comparing RPC errors, as I'm not sure how bitcoind
converts them internally, so I can only rely on the end results from its rpc responses,
- https://github.com/bitcoin/bitcoin/blob/master/test/functional/data/invalid_txs.py
- https://github.com/bitcoin/bitcoin/blob/master/test/functional/mempool_accept.py
Also updated the link, and added a new method to properly handle error matching. Meanwhile plan to work on something in btcd/btcjson
- this way we can have a single place to track the bitcoind errors and handles the matching then turning them into defined error types.
8d0af32
to
956cc29
Compare
cc: @ellemouton for review |
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.
lgtm!
956cc29
to
38c00dc
Compare
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.
LGTM 🍭
TestMempoolAccept
is now added to chain interface so it can be accessed via chain io inlnd
.