Simplify Base32 and Base64 conversions #11630

1. 
   sipa commented at 10:26 PM on November 7, 2017: member

   Generalize ConvertBits a bit to also be usable for the existing Base32 and Base64 convertions (rather than just for Bech32).

2. 
   meshcollider commented at 10:46 PM on November 7, 2017: contributor

   Impressive simplification, Concept ACK

3. fanquake added the label Refactoring on Nov 7, 2017
4. 
   fanquake commented at 12:33 AM on November 8, 2017: member

   All Travis failures seem to be the same issue in signmessages.py

       self.run_test()
     File "/home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-unknown-linux-gnu/test/functional/signmessages.py", line 24, in run_test
       assert(self.nodes[0].verifymessage(address, signature, message))
     File "/home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-unknown-linux-gnu/test/functional/test_framework/coverage.py", line 47, in __call__
       return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
     File "/home/travis/build/bitcoin/bitcoin/build/bitcoin-x86_64-unknown-linux-gnu/test/functional/test_framework/authproxy.py", line 138, in __call__
       raise JSONRPCException(response['error'])
   test_framework.authproxy.JSONRPCException: Malformed base64 encoding (-5)
   2017-11-07 23:50:01.876000 TestFramework (INFO): Stopping nodes
   2017-11-07 23:50:01.877000 TestFramework.node0 (DEBUG): Stopping node
   

5. sipa force-pushed on Nov 8, 2017
6. 
   sipa commented at 5:14 PM on November 8, 2017: member

   @fanquake Thanks, fixed.

7. in src/utilstrencodings.cpp:231 in 8059735f50 outdated

    286 | -                 vchRet.push_back((left<<5) | dec);
    287 | -                 mode = 0;
    288 | -                 break;
    289 | -         }
    290 | +    const char* e = p;
    291 | +    std::vector<uint8_t> val;
   

   ---

   ---

   

   promag commented at 8:40 PM on November 8, 2017:

   Any reason to not reserve? Doesn't pay off?

8. in src/utilstrencodings.cpp:252 in 8059735f50 outdated

    336 | +            valid = false;
    337 | +            break;
    338 |          }
    339 | +        ++p;
    340 | +    }
    341 | +    valid = valid && (p - e) % 8 == 0 && p - q < 8;
   

   ---

   ---

   

   promag commented at 10:53 PM on November 8, 2017:

   Not sure but isn't (p - e) & 7 == 0 faster? Or does the compiler optimise this case?

   ---

   

   sipa commented at 2:04 AM on November 17, 2017:

   Strength reduction will do that for you at -O1 and higher.

9. sipa force-pushed on Nov 23, 2017
10. 
    TheBlueMatt commented at 5:18 PM on December 8, 2017: member

    utACK 16a01bf1a2a318effbbac4a22f1092fee1924dc8. Would be nice to add these folks to test_bitcoin_fuzzy.cpp!

11. 
    laanwj commented at 10:59 PM on March 1, 2018: member

    utACK. Needs rebase in src/utilstrencodings.cpp.

12. laanwj added this to the milestone 0.17.0 on Mar 5, 2018
13. sipa force-pushed on Mar 5, 2018
14. 
    sipa commented at 9:57 PM on March 5, 2018: member

    Rebased.

15. in src/utilstrencodings.cpp:165 in eb59db5a30 outdated

     197 | -                 break;
     198 | -         }
     199 | +    const char* e = p;
     200 | +    std::vector<uint8_t> val;
     201 | +    while (*p != 0) {
     202 | +        int x = decode64_table[(unsigned char)*p];
    

    ---

    ---

    

    laanwj commented at 10:19 PM on March 5, 2018:

    It seems a bit of a pity to allocate and fill an intermediate vector here, which is just the contents of the string mapped through decode64_table. Could we maybe wrap the iterator to weave this into ConvertBits? (to be clear, this could be done later, no need to do so in this PR, but as the rest of the changes is so elegant it just jumped out)

    ---

    

    sipa commented at 4:31 AM on March 7, 2018:

    @laanwj I actually tried that, and it became a bit more complex than I wanted to do in this PR.

    ---

    

    laanwj commented at 1:51 PM on March 7, 2018:

    Yes it seems quite annoying to do so in C++, iterators are tricky enough to make this seemingly trivial thing non-trivial :/

    So as we keep it like this, we should heed @promag's suggestion and reserve, I think.

16. 
    laanwj commented at 11:10 PM on March 6, 2018: member

    Needs rebase after #11372

17. Generalize ConvertBits 3296a3bb7f
18. sipa force-pushed on Mar 7, 2018
19. 
    sipa commented at 4:30 AM on March 7, 2018: member

    Rebased.

20. sipa force-pushed on Mar 7, 2018
21. 
    sipa commented at 2:58 PM on March 7, 2018: member

    Added reserves in several places.

22. Simplify Base32 and Base64 conversions b3ea8ccb7a
23. sipa force-pushed on Mar 7, 2018
24. 
    laanwj commented at 3:23 PM on March 7, 2018: member

    Added reserves in several places.

    Thanks!

    utACK b3ea8ccb7af475703b97246a2baf2e105d24d6f9

25. laanwj merged this on Mar 7, 2018
26. laanwj closed this on Mar 7, 2018

    ---

27. laanwj referenced this in commit da9a2f5cd9 on Mar 7, 2018
28. zkbot referenced this in commit 771aee5d88 on Jun 11, 2018
29. zkbot referenced this in commit 2ebde5860e on Jun 19, 2018
30. deadalnix referenced this in commit c8254da60e on Apr 16, 2020
31. deadalnix referenced this in commit 9fa8a6f08e on Apr 16, 2020
32. ftrader referenced this in commit 89c22648bc on Aug 17, 2020
33. ftrader referenced this in commit e37bdbcc06 on Aug 17, 2020
34. UdjinM6 referenced this in commit ae0d180ef5 on Dec 25, 2020
35. UdjinM6 referenced this in commit fbb40717b4 on Jan 8, 2021
36. MarcoFalke locked this on Sep 8, 2021

Contributors

sipameshcolliderfanquakepromagTheBlueMattlaanwj

Milestone
0.17.0