Load Base64 PSBT string from file #469

1. 
   achow101 commented at 9:31 pm on November 12, 2021: member

   Some .psbt files may have the PSBT as a base64 string instead of in binary. We should be able to load those files.
2. 
   hebasto commented at 9:53 pm on November 12, 2021: member

   Some .psbt files may have the PSBT as a base64 string instead of in binary.

   How to reliably create a correct such file?

3. hebasto renamed this:
   gui: Load Base64 PSBT string from file
   Load Base64 PSBT string from file
   on Nov 12, 2021
4. 
   achow101 commented at 9:55 pm on November 12, 2021: member

   How to reliably create a correct such file?

   Get Base64 PSBT string. Make a new file. Paste string into file.

5. hebasto added the label Feature on Nov 21, 2021
6. in src/qt/walletframe.cpp:223 in d54f498f8a outdated

   211@@ -212,6 +212,15 @@ void WalletFrame::gotoLoadPSBT(bool from_clipboard)
   212         }
   213         std::ifstream in(filename.toLocal8Bit().data(), std::ios::binary);
   214         data = std::string(std::istreambuf_iterator<char>{in}, {});
   215+
   216+        // Some psbt files may be base64 strings in the file rather than binary data
   217+        bool invalid = false;
   218+        std::string b64_dec = data;
   219+        b64_dec.erase(b64_dec.find_last_not_of(" \t\n\r\f\v") + 1); // Trim trailing whitespace
   220+        b64_dec = DecodeBase64(b64_dec, &invalid);
   

   ---

   ---

   

   luke-jr commented at 11:34 pm on November 30, 2021:

   Base64 files standardly have newlines every N charaters, but our DecodeBase64 doesn’t tolerate them currently…

   Nevermind, the BIP specifies a RFC that doesn’t allow newlines

7. jarolrod approved
8. 
   jarolrod commented at 4:37 am on December 23, 2021: member

   tACK d54f498f8abbc49ff179e965e421cb0d8d5f5baa

   Tested by saving a base64 psbt using sparrow wallet. Loading the file fails on master with Unable to decode PSBT Invalid PSBT magic bytes: iostream error. On this PR branch, i can load the PSBT successfully.

9. in src/qt/walletframe.cpp:222 in d54f498f8a outdated

   211@@ -212,6 +212,15 @@ void WalletFrame::gotoLoadPSBT(bool from_clipboard)
   212         }
   213         std::ifstream in(filename.toLocal8Bit().data(), std::ios::binary);
   214         data = std::string(std::istreambuf_iterator<char>{in}, {});
   215+
   216+        // Some psbt files may be base64 strings in the file rather than binary data
   217+        bool invalid = false;
   218+        std::string b64_dec = data;
   219+        b64_dec.erase(b64_dec.find_last_not_of(" \t\n\r\f\v") + 1); // Trim trailing whitespace
   

   ---

   ---

   

   promag commented at 0:42 am on February 23, 2022:

   We have Trim in src/util/string.h:

   0bool invalid;
   1const auto decoded = DecodeBase64(Trim(data), &invalid);
   2if (!invalid) data = decoded
   

10. 
    promag commented at 0:43 am on February 23, 2022: contributor

    Concept ACK.
11. 
    jb55 commented at 4:30 am on February 23, 2022: contributor

    Concept ACK
12. DrahtBot added the label Needs rebase on Apr 27, 2022
13. achow101 force-pushed on Apr 27, 2022
14. DrahtBot removed the label Needs rebase on Apr 28, 2022
15. in src/qt/walletframe.cpp:227 in 78cfec3bf4 outdated

    222+        b64_dec.erase(b64_dec.find_last_not_of(" \t\n\r\f\v") + 1); // Trim trailing whitespace
    223+        b64_dec = DecodeBase64(b64_dec, &invalid);
    224+        if (!invalid) {
    225+            data = b64_dec;
    226+        }
    227+>>>>>>> d54f498f8 (gui: Load Base64 PSBT string from file)
    

    ---

    ---

    

    luke-jr commented at 3:25 am on April 28, 2022:

    o.O

    ---

    

    achow101 commented at 3:27 am on April 28, 2022:

    Oops. fixed.
16. achow101 force-pushed on Apr 28, 2022
17. gui: Load Base64 PSBT string from file

    Some .psbt files may have the PSBT as a base64 string instead of in
    binary. We should be able to load those files.

    2c3ee4c347
18. achow101 force-pushed on Apr 28, 2022
19. 
    DrahtBot commented at 9:48 pm on June 12, 2022: contributor

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    Conflicts

    No conflicts as of last run.

20. 
    jarolrod commented at 5:04 am on June 28, 2022: member

    tACK 2c3ee4c347838ecadb17a011932dffc077e46630

    Tested with same method as stated here: #469#pullrequestreview-839000783

    Could this use a release note?

21. in src/qt/walletframe.cpp:219 in 2c3ee4c347

    214@@ -215,6 +215,14 @@ void WalletFrame::gotoLoadPSBT(bool from_clipboard)
    215         }
    216         std::ifstream in{filename.toLocal8Bit().data(), std::ios::binary};
    217         data.assign(std::istream_iterator<unsigned char>{in}, {});
    218+
    219+        // Some psbt files may be base64 strings in the file rather than binary data
    

    ---

    ---

    

    laanwj commented at 8:05 am on June 28, 2022:

    I dislike this kind of ambiguity in file formats, it’s a thing that can bite you in unexpected ways at some point, but I understand the reason. A valid binary PSBT can never be valid b64, I hope?

    ---

    

    achow101 commented at 2:36 pm on July 6, 2022:

    A valid binary PSBT can never be valid b64, I hope?

    Yes. The magic in binary are the characters psbt, but are cHn... in base64. Interpreting the binary as base64 or vice versa would result in the wrong magic.

22. shaavan approved
23. 
    shaavan commented at 12:34 pm on July 6, 2022: contributor

    ACK 2c3ee4c347838ecadb17a011932dffc077e46630

    • I was able to load the Base64 PSBT transaction from the file through this PR, which the master failed to do so.
24. hebasto merged this on Jul 15, 2022
25. hebasto closed this on Jul 15, 2022

    ---

26. sidhujag referenced this in commit 4ce9979450 on Jul 18, 2022
27. bitcoin-core locked this on Jul 15, 2023

Contributors
achow101 hebasto luke-jr jarolrod promag jb55 DrahtBot laanwj shaavan

Labels
Feature