check that a separator is found for psbt inputs, outputs, and global map #14377

1. 
   achow101 commented at 4:28 am on October 3, 2018: member

   Currently it doesn’t make sure that a separator was found so PSBTs missing a trailing separator would still pass. This fixes that and adds a test case for it.

   It really only makes sense to check for the separator for the output maps as if an input or global map was missing a separator, the fields following it would be interpreted as belonging to the previous input or global map. However I have added the check for those two anyways to be consistent.

2. check that a separator is found for psbt inputs, outputs, and global map 4fb3388db9
3. 
   MarcoFalke commented at 4:37 am on October 3, 2018: member

   Is this for 0.17.1 backport?
4. MarcoFalke added the label RPC/REST/ZMQ on Oct 3, 2018
5. in src/script/sign.h:313 in 4fb3388db9

   309             // the key is empty if that was actually a separator byte
   310             // This is a special case for key lengths 0 as those are not allowed (except for separator)
   311-            if (key.empty()) return;
   312+            if (key.empty()) {
   313+                found_sep = true;
   314+                break;
   

   ---

   ---

   

   jimmysong commented at 2:45 pm on October 3, 2018:

   What’s the rationale for breaking here instead of throwing the error right away?

   ---

   

   achow101 commented at 3:02 pm on October 3, 2018:

   We want to throw the error if we don’t get here.
6. in src/script/sign.h:310 in 4fb3388db9

   305             // Read
   306             std::vector<unsigned char> key;
   307             s >> key;
   308 
   309             // the key is empty if that was actually a separator byte
   310             // This is a special case for key lengths 0 as those are not allowed (except for separator)
   

   ---

   ---

   

   jimmysong commented at 2:46 pm on October 3, 2018:

   Comment may need updating?

   ---

   

   achow101 commented at 3:02 pm on October 3, 2018:

   No
7. in src/script/sign.h:485 in 4fb3388db9

   489             std::vector<unsigned char> key;
   490             s >> key;
   491 
   492             // the key is empty if that was actually a separator byte
   493             // This is a special case for key lengths 0 as those are not allowed (except for separator)
   494-            if (key.empty()) return;
   

   ---

   ---

   

   jimmysong commented at 2:46 pm on October 3, 2018:

   same as above, comment may need updating?
8. in src/script/sign.h:612 in 4fb3388db9

   624             std::vector<unsigned char> key;
   625             s >> key;
   626 
   627             // the key is empty if that was actually a separator byte
   628             // This is a special case for key lengths 0 as those are not allowed (except for separator)
   629-            if (key.empty()) break;
   

   ---

   ---

   

   jimmysong commented at 2:47 pm on October 3, 2018:

   comment may need updating?
9. 
   sipa commented at 1:37 am on October 4, 2018: member

   utACK 4fb3388db95f408566e43ebb9736842cfbff0a7d. I think it should go into 0.17.1.
10. MarcoFalke added this to the milestone 0.17.1 on Oct 4, 2018
11. MarcoFalke added the label Needs backport on Oct 4, 2018
12. 
    DrahtBot commented at 6:04 am on October 4, 2018: member

    Coverage	Change (pull 14377)	Reference (master)
    Lines	-0.0117 %	87.0471 %
    Functions	-0.0154 %	84.1130 %
    Branches	+0.0018 %	51.5403 %

13. 
    meshcollider commented at 2:35 pm on October 12, 2018: contributor

    utACK https://github.com/bitcoin/bitcoin/pull/14377/commits/4fb3388db95f408566e43ebb9736842cfbff0a7d
14. laanwj merged this on Nov 1, 2018
15. laanwj closed this on Nov 1, 2018

    ---

16. laanwj referenced this in commit 51e5ef3971 on Nov 1, 2018
17. 
    fanquake commented at 2:28 pm on November 30, 2018: member

    This will be backported in #14780.
18. fanquake removed the label Needs backport on Nov 30, 2018
19. sipa referenced this in commit a3fe125490 on Dec 3, 2018
20. MarcoFalke referenced this in commit 5d12143c73 on Dec 5, 2018
21. deadalnix referenced this in commit 0bf843494d on Oct 5, 2020
22. Munkybooty referenced this in commit 4bea828574 on Jul 22, 2021
23. Munkybooty referenced this in commit a84a7af43b on Jul 22, 2021
24. Munkybooty referenced this in commit 4444962d69 on Jul 22, 2021
25. Munkybooty referenced this in commit 08eab73491 on Jul 23, 2021
26. Munkybooty referenced this in commit 023ee99fcb on Jul 23, 2021
27. Munkybooty referenced this in commit 705ccf1c8d on Jul 23, 2021
28. Munkybooty referenced this in commit d841d5a422 on Jul 23, 2021
29. Munkybooty referenced this in commit 49fcdace99 on Jul 26, 2021
30. Munkybooty referenced this in commit acd8f66531 on Jul 27, 2021
31. Munkybooty referenced this in commit 65c030f7ac on Jul 28, 2021
32. DrahtBot locked this on Sep 8, 2021

Contributors
achow101 MarcoFalke jimmysong sipa DrahtBot meshcollider fanquake

Labels
RPC/REST/ZMQ

Milestone
0.17.1