streams: Accept URef obj for VectorReader unserialize #21581

1. 
   MarcoFalke commented at 11:38 AM on April 3, 2021: member

   Missed in commit 172f5fa738d419efda99542e2ad2a0f4db5be580. An URef may collapse into an LRef or RRef depending on context. There is no reason to forbid RRef in VectorReader::operator>>, so add it for consistency.

2. MarcoFalke added the label Refactoring on Apr 3, 2021
3. MarcoFalke requested review from sipa on May 5, 2021
4. MarcoFalke requested review from ryanofsky on May 5, 2021
5. in src/test/streams_tests.cpp:114 in faadb43546 outdated

    109 | @@ -110,6 +110,9 @@ BOOST_AUTO_TEST_CASE(streams_vector_reader)
    110 |      // Reading after end of byte vector throws an error even if the reader is
    111 |      // not totally empty.
    112 |      BOOST_CHECK_THROW(new_reader >> d, std::ios_base::failure);
    113 | +
    114 | +    // Serialize into r-value
   

   ---

   ---

   

   ryanofsky commented at 5:54 PM on May 5, 2021:

   In commit "streams: Accept URef obj for VectorReader unserialize" (faadb43546d7befc2a28d5bffa461efc26103cbd)

   s/Serialize/Deserialize

   ---

   

   MarcoFalke commented at 6:22 PM on May 5, 2021:

   Thanks, took your suggested test case

6. ryanofsky approved
7. 
   ryanofsky commented at 5:55 PM on May 5, 2021: member

   Code review ACK faadb43546d7befc2a28d5bffa461efc26103cbd

   Good catch! I would maybe expand the test to motivate this a little more. Suggestion:

   BOOST_AUTO_TEST_CASE(streams_vector_reader_rvalue)
   {
       std::vector<unsigned char> data = {0x82, 0xa7, 0x31};
       VectorReader reader(SER_NETWORK, INIT_PROTO_VERSION, data, /* pos= */ 0);
       uint32_t varint = 0;
       // Deserialize into r-value
       reader >> VARINT(varint);
       BOOST_CHECK_EQUAL(varint, 54321);
       BOOST_CHECK(reader.empty());
   }
   

8. MarcoFalke force-pushed on May 5, 2021
9. streams: Accept URef obj for VectorReader unserialize fa2204f6ad
10. MarcoFalke force-pushed on May 5, 2021
11. ryanofsky approved
12. 
    ryanofsky commented at 7:20 PM on May 5, 2021: member

    Code review ACK fa2204f6adef493079d1ca5148b0fdc2b55816e6, just expanded test since last review

13. MarcoFalke merged this on May 10, 2021
14. MarcoFalke closed this on May 10, 2021

    ---

15. MarcoFalke deleted the branch on May 10, 2021
16. sidhujag referenced this in commit 80f770388b on May 10, 2021
17. PastaPastaPasta referenced this in commit 068d09ed40 on Jun 27, 2021
18. PastaPastaPasta referenced this in commit 0f15b74e16 on Jun 28, 2021
19. PastaPastaPasta referenced this in commit 1aa8dbb27c on Jun 29, 2021
20. PastaPastaPasta referenced this in commit 73e63b89e9 on Jul 1, 2021
21. PastaPastaPasta referenced this in commit 34a9f91f7b on Jul 1, 2021
22. PastaPastaPasta referenced this in commit 34c414e8db on Jul 15, 2021
23. gwillen referenced this in commit b30d96864f on Jun 1, 2022
24. DrahtBot locked this on Aug 18, 2022

Contributors

MarcoFalkeryanofsky

Review Requested
sipa