After hard fork, replay protected tx's require the amount field to be non-zero for previous outputs in signed tx's. JSON RPC code was accepting transactions with this field missing when it should have been throwing an error. Users had the impression the transaction that was signed was good, but when trying to sent to network, they would get an error that the tx is bad. See github issue https://github.com/Bitcoin-ABC/bitcoin-abc/issues/63.
Details
Details
- Reviewers
freetrader dgenr8 deadalnix sickpig - Group Reviewers
Restricted Project
make check and also rpc-tests.py
Diff Detail
Diff Detail
- Repository
- rABC Bitcoin ABC
- Branch
- master
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 696 Build 696: arc lint + arc unit
Event Timeline
Comment Actions
@dgenr8: As per your comment in the github issue 63:
Yes, though I've only run it in test. It was previously failing on broadcast, because verification expects the actual amount (not 0) to be signed.
Should I be disallowing '0' values for 'amount' in the C++ signrawtransaction JSON function ? Please advise!
Comment Actions
It would make the review process easier if you will apply format changes in another diff.
Once the formatting diff will be merged, you could rebase this one.
Comment Actions
@sickpig : I *agree* with you. The thing is I'm between a rock and a hard place. If I don't let the 'arcanist' auto-linter apply the formatting changes, @deadalnix won't accept it because the code has "linter" code style errors. If I allow the linter to run, it produces this huge diff that has formatting stuff mixed in with logic changes.
So you want me to create *two* diffs? One with just formatting (autopep8) stuff and one with my changes?
Comment Actions
So you want me to create *two* diffs? One with just formatting (autopep8) stuff and one with my changes?
yes.
I suppose your are using a local branch cloned from master to dev this feature, right?
if it is the case create a new branch based on master, apply the formatting change, arc diff.
The new one will be quickly accepted by @deadalnix (and me fwiw), then you can land it. Once landed (merged) just merge/rebase master onto your local branch, fix conflicts (if any), and then run arc diff to update this diff.