Fix Preprepare sizing issues (#2649)

* **Problem Overview**  
MessageBase considers the span size outside of the regular "size" parameter. Hence, in some cases, we may end up letting a message be larger than the allowed size.  For example, if a prePrepare message span size is smaller than the max span size, we may end up with a message that exceeds a bit from the maximal size. This change is for fixing this issue
* **Testing Done**  
  CI tests

Author: Yehonatan Buchnik

bftengine/src/bftengine/messages/PrePrepareMsg.cpp

@@ -151,12 +151,13 @@ PrePrepareMsg::PrePrepareMsg(ReplicaId sender,
                              const concordUtils::SpanContext& spanContext,
                              const std::string& batchCid,
                              size_t size)
-    : MessageBase(sender,
-                  MsgCode::PrePrepare,
-                  spanContext.data().size(),
-                  (((size + sizeof(Header) + batchCid.size()) < maxMessageSize<PrePrepareMsg>())
-                       ? (size + sizeof(Header) + batchCid.size())
-                       : maxMessageSize<PrePrepareMsg>() - spanContext.data().size())) {
+    : MessageBase(
+          sender,
+          MsgCode::PrePrepare,
+          spanContext.data().size(),
+          (((size + sizeof(Header) + batchCid.size()) + spanContext.data().size() < maxMessageSize<PrePrepareMsg>())
+               ? (size + sizeof(Header) + batchCid.size())
+               : maxMessageSize<PrePrepareMsg>() - spanContext.data().size())) {
   bool ready = size == 0;  // if null, then message is ready
   if (!ready) {
     b()->digestOfRequests.makeZero();

bftengine/src/bftengine/messages/PrePrepareMsg.hpp

@@ -155,7 +155,7 @@ class RequestsIterator {
 
 template <>
 inline MsgSize maxMessageSize<PrePrepareMsg>() {
-  return ReplicaConfig::instance().getmaxExternalMessageSize() + MessageBase::SPAN_CONTEXT_MAX_SIZE;
+  return ReplicaConfig::instance().getmaxExternalMessageSize();
 }
 
 }  // namespace impl

bftengine/tests/messages/PrePrepareMsg_test.cpp

@@ -229,6 +229,35 @@ TEST_F(PrePrepareMsgTestFixture, base_methods) {
   testMessageBaseMethods(msg, MsgCode::PrePrepare, senderId, spanContext);
 }
 
+TEST_F(PrePrepareMsgTestFixture, test_prePrepare_size) {
+  bftEngine::ReservedPagesClientBase::setReservedPages(&res_pages_mock_);
+  ReplicasInfo replicaInfo(createReplicaConfig(), false, false);
+  ReplicaId senderId = 1u;
+  ViewNum viewNum = 2u;
+  SeqNum seqNum = 3u;
+  CommitPath commitPath = CommitPath::OPTIMISTIC_FAST;
+  // Create random span
+  uint span_size = rand() % 1024;
+  const std::string spanContext = getRandomStringOfLength(span_size);
+  PrePrepareMsg msg(senderId,
+                    viewNum,
+                    seqNum,
+                    commitPath,
+                    concordUtils::SpanContext{spanContext},
+                    config.getmaxExternalMessageSize());
+  std::vector<std::shared_ptr<ClientRequestMsg>> client_request;
+
+  while (true) {
+    client_request.clear();
+    create_random_client_requests(client_request, 1u);
+    if (client_request.front()->size() > msg.remainingSizeForRequests()) break;
+    msg.addRequest(client_request.front()->body(), client_request.front()->size());
+  }
+  msg.finishAddingRequests();
+  EXPECT_NO_THROW(msg.validate(replicaInfo));
+  ASSERT_TRUE(msg.size() <= config.getmaxExternalMessageSize());
+}
+
 int main(int argc, char** argv) {
   ::testing::InitGoogleTest(&argc, argv);
   return RUN_ALL_TESTS();