Write PSBTs to file with binary mode #188

1. 
   achow101 commented at 1:57 am on January 19, 2021: member

   As noted in https://github.com/bitcoin/bitcoin/issues/20959, PSBT files should be opened in binary mode as on windows, all newlines are turned into CRLF which produces invalid PSBTs.

   Fixes https://github.com/bitcoin/bitcoin/issues/20959

2. GUI: Write PSBTs to file with binary mode cc3971c9ff
3. MarcoFalke added this to the milestone 0.21.1 on Jan 19, 2021
4. MarcoFalke added the label windows on Jan 21, 2021
5. MarcoFalke renamed this:
   GUI: Write PSBTs to file with binary mode
   Write PSBTs to file with binary mode
   on Jan 21, 2021
6. 
   MarcoFalke commented at 6:03 pm on January 21, 2021: contributor

   Approach ACK. Is there a way to test this on non-windows?
7. 
   hebasto commented at 5:45 pm on February 22, 2021: member

   Concept ACK.

   For reference: https://en.cppreference.com/w/cpp/io/c/FILE#Binary_and_text_modes

8. 
   hebasto commented at 5:49 pm on February 22, 2021: member

   @Quantris Does this pr fix https://github.com/bitcoin/bitcoin/issues/20959 for you?
9. Talkless approved
10. 
    Talkless commented at 6:17 pm on March 11, 2021: none

    utACK cc3971c9ff538a924c1a76ca1352bcaeb24f579f.

    I DID test, but only on Linux (Debian Sid with Qt 5.15.2). I’ve created .psbt with PR version, imported into master version, and was able to sing and broadcast (regtest), so at least I don’t see regressions in Linux.

11. MarcoFalke merged this on Mar 11, 2021
12. MarcoFalke closed this on Mar 11, 2021

    ---

13. MarcoFalke referenced this in commit 3a12672419 on Mar 11, 2021
14. 
    MarcoFalke commented at 8:16 pm on March 11, 2021: contributor

    Backported in https://github.com/bitcoin/bitcoin/pull/20901
15. sidhujag referenced this in commit 92e4f92429 on Mar 11, 2021
16. 
    Quantris commented at 9:25 am on June 10, 2021: none

    @Quantris Does this pr fix bitcoin/bitcoin#20959 for you?

    Sorry I missed replying to this at the time. I’m not really set up to build the project myself so I wouldn’t have been able to test this without someone providing a binary.

    I can affirm that the issue seems fixed in 0.21.1 which includes this change; I tested in Win10 with the following steps in a watch-only wallet:

    1. create a partial transaction & save to file test.psbt (during this process it is also copied to clipboard in base64)
    2. load from clipboard, confirm it displays as expected & save to another file test2.psbt
    3. load from file test.psbt, confirm it displays as expected
    4. load from file test2.psbt, confirm it displays as expected

    The same process in 0.21.0 fails in steps 3 & 4.

    I also looked around a bit for info on testing this kind of issue. Unfortunately I didn’t find anything that makes text-mode vs. binary-mode distinguishable in linux (I was thinking maybe there would be some kind of env var or build flag for the stdlib). IOW seems like the only way to test this would be to run the tests in a Windows env.

    Thanks for the fix @achow101 !

17. apoelstra referenced this in commit b27e9074ee on Sep 21, 2021
18. apoelstra referenced this in commit 2b372d3ab3 on Sep 21, 2021
19. apoelstra referenced this in commit 337297d605 on Sep 22, 2021
20. gwillen referenced this in commit 4135bed184 on Jun 28, 2022
21. bitcoin-core locked this on Aug 16, 2022

Contributors
achow101 MarcoFalke hebasto Talkless Quantris

Labels
Windows

Milestone
0.21.1