Merge pull request #4104 from dmkozh/diag_flag

Use a separate flag for validation diagnostics in tx endpoint.

Reviewed-by: sisuresh

src/main/CommandHandler.cpp

@@ -998,7 +998,8 @@ CommandHandler::tx(std::string const& params, std::string& retStr)
                                      1);
                 resultBase64 = decoder::encode_b64(resultBin);
                 root["error"] = resultBase64;
-                if (mApp.getConfig().ENABLE_SOROBAN_DIAGNOSTIC_EVENTS &&
+                if (mApp.getConfig().ENABLE_DIAGNOSTICS_FOR_TX_SUBMISSION &&
+                    transaction->isSoroban() &&
                     !transaction->getDiagnosticEvents().empty())
                 {
                     auto diagsBin =

src/main/Config.cpp

@@ -264,6 +264,7 @@ Config::Config() : NODE_SEED(SecretKey::random())
     MAX_DEX_TX_OPERATIONS_IN_TX_SET = std::nullopt;
 
     ENABLE_SOROBAN_DIAGNOSTIC_EVENTS = false;
+    ENABLE_DIAGNOSTICS_FOR_TX_SUBMISSION = false;
     TESTING_MINIMUM_PERSISTENT_ENTRY_LIFETIME = 0;
     TESTING_SOROBAN_HIGH_LIMIT_OVERRIDE = false;
     OVERRIDE_EVICTION_PARAMS_FOR_TESTING = false;

src/main/Config.h

@@ -557,13 +557,21 @@ class Config : public std::enable_shared_from_this<Config>
     // The default value is false.
     bool HALT_ON_INTERNAL_TRANSACTION_ERROR;
 
-    // If set to true, env will return additional diagnostic Soroban events
-    // that are not part of the protocol. These events will be put into a list
-    // in the non-hashed portion of the meta, and this list will contain all
-    // events so ordering can be maintained between all events. The default
-    // value is false, and this should not be enabled on validators.
+    // If set to true, additional diagnostic Soroban events that are not part
+    // of the protocol will be generated while applying Soroban transactions.
+    // These events will be put into a list in the non-hashed portion of the
+    // meta, and this list will contain all events so ordering can be
+    // maintained between all events. The default value is false, and this
+    // should not be enabled on validators.
     bool ENABLE_SOROBAN_DIAGNOSTIC_EVENTS;
 
+    // If set to true, attach a small diagnostics message in the format of
+    // Soroban diagnostic event to some transaction submission errors (mainly,
+    // `txSOROBAN_INVALID` errors). The diagnostics message is guaranteed to be
+    // small and independent of the user input, so this can be safely enabled
+    // on validators that accept transactions.
+    bool ENABLE_DIAGNOSTICS_FOR_TX_SUBMISSION;
+
     // Override the initial hardcoded MINIMUM_PERSISTENT_ENTRY_LIFETIME
     // for testing.
     uint32_t TESTING_MINIMUM_PERSISTENT_ENTRY_LIFETIME;

src/simulation/LoadGenerator.cpp

@@ -1240,7 +1240,11 @@ LoadGenerator::createContractTransaction(uint32_t ledgerNum, uint64_t accountId,
     createResources.readBytes = mContactOverheadBytes;
     createResources.writeBytes = 300;
 
-    auto salt = sha256(std::to_string(mContractInstanceKeys.size()));
+    auto salt = sha256(
+        std::to_string(mContractInstanceKeys.size()) + "run" +
+        std::to_string(mApp.getMetrics()
+                           .NewMeter({"loadgen", "run", "complete"}, "run")
+                           .count()));
     auto contractIDPreimage = makeContractIDPreimage(*account, salt);
 
     auto tx =

src/transactions/ExtendFootprintTTLOpFrame.cpp

@@ -120,7 +120,7 @@ ExtendFootprintTTLOpFrame::doApply(Application& app, AbstractLedgerTxn& ltx,
 
         if (resources.readBytes < metrics.mLedgerReadByte)
         {
-            mParentTx.pushSimpleDiagnosticError(
+            mParentTx.pushApplyTimeDiagnosticError(
                 app.getConfig(), SCE_BUDGET, SCEC_EXCEEDED_LIMIT,
                 "operation byte-read resources exceeds amount specified",
                 {makeU64SCVal(metrics.mLedgerReadByte),
@@ -168,7 +168,7 @@ ExtendFootprintTTLOpFrame::doCheckValid(
     if (!footprint.readWrite.empty())
     {
         innerResult().code(EXTEND_FOOTPRINT_TTL_MALFORMED);
-        mParentTx.pushSimpleDiagnosticError(
+        mParentTx.pushValidationTimeDiagnosticError(
             appConfig, SCE_STORAGE, SCEC_INVALID_INPUT,
             "read-write footprint must be empty for ExtendFootprintTTL "
             "operation",
@@ -181,7 +181,7 @@ ExtendFootprintTTLOpFrame::doCheckValid(
         if (!isSorobanEntry(lk))
         {
             innerResult().code(EXTEND_FOOTPRINT_TTL_MALFORMED);
-            mParentTx.pushSimpleDiagnosticError(
+            mParentTx.pushValidationTimeDiagnosticError(
                 appConfig, SCE_STORAGE, SCEC_INVALID_INPUT,
                 "only entries with TTL (contract data or code entries) can "
                 "have it extended",
@@ -194,7 +194,7 @@ ExtendFootprintTTLOpFrame::doCheckValid(
         networkConfig.stateArchivalSettings().maxEntryTTL - 1)
     {
         innerResult().code(EXTEND_FOOTPRINT_TTL_MALFORMED);
-        mParentTx.pushSimpleDiagnosticError(
+        mParentTx.pushValidationTimeDiagnosticError(
             appConfig, SCE_STORAGE, SCEC_INVALID_INPUT,
             "TTL extension is too large: {} > {}",
             {

src/transactions/InvokeHostFunctionOpFrame.cpp

@@ -457,7 +457,7 @@ InvokeHostFunctionOpFrame::doApply(Application& app, AbstractLedgerTxn& ltx,
 
             if (resources.readBytes < metrics.mLedgerReadByte)
             {
-                mParentTx.pushSimpleDiagnosticError(
+                mParentTx.pushApplyTimeDiagnosticError(
                     appConfig, SCE_BUDGET, SCEC_EXCEEDED_LIMIT,
                     "operation byte-read resources exceeds amount specified",
                     {makeU64SCVal(metrics.mLedgerReadByte),
@@ -536,7 +536,7 @@ InvokeHostFunctionOpFrame::doApply(Application& app, AbstractLedgerTxn& ltx,
         }
         if (resources.instructions < out.cpu_insns)
         {
-            mParentTx.pushSimpleDiagnosticError(
+            mParentTx.pushApplyTimeDiagnosticError(
                 appConfig, SCE_BUDGET, SCEC_EXCEEDED_LIMIT,
                 "operation instructions exceeds amount specified",
                 {makeU64SCVal(out.cpu_insns),
@@ -545,7 +545,7 @@ InvokeHostFunctionOpFrame::doApply(Application& app, AbstractLedgerTxn& ltx,
         }
         else if (sorobanConfig.txMemoryLimit() < out.mem_bytes)
         {
-            mParentTx.pushSimpleDiagnosticError(
+            mParentTx.pushApplyTimeDiagnosticError(
                 appConfig, SCE_BUDGET, SCEC_EXCEEDED_LIMIT,
                 "operation memory usage exceeds network config limit",
                 {makeU64SCVal(out.mem_bytes),
@@ -586,7 +586,7 @@ InvokeHostFunctionOpFrame::doApply(Application& app, AbstractLedgerTxn& ltx,
             metrics.noteWriteEntry(isCodeKey(lk), keySize, entrySize);
             if (resources.writeBytes < metrics.mLedgerWriteByte)
             {
-                mParentTx.pushSimpleDiagnosticError(
+                mParentTx.pushApplyTimeDiagnosticError(
                     appConfig, SCE_BUDGET, SCEC_EXCEEDED_LIMIT,
                     "operation byte-write resources exceeds amount specified",
                     {makeU64SCVal(metrics.mLedgerWriteByte),
@@ -661,7 +661,7 @@ InvokeHostFunctionOpFrame::doApply(Application& app, AbstractLedgerTxn& ltx,
         if (sorobanConfig.txMaxContractEventsSizeBytes() <
             metrics.mEmitEventByte)
         {
-            mParentTx.pushSimpleDiagnosticError(
+            mParentTx.pushApplyTimeDiagnosticError(
                 appConfig, SCE_BUDGET, SCEC_EXCEEDED_LIMIT,
                 "total events size exceeds network config maximum",
                 {makeU64SCVal(metrics.mEmitEventByte),
@@ -679,7 +679,7 @@ InvokeHostFunctionOpFrame::doApply(Application& app, AbstractLedgerTxn& ltx,
     metrics.mEmitEventByte += static_cast<uint32>(out.result_value.data.size());
     if (sorobanConfig.txMaxContractEventsSizeBytes() < metrics.mEmitEventByte)
     {
-        mParentTx.pushSimpleDiagnosticError(
+        mParentTx.pushApplyTimeDiagnosticError(
             appConfig, SCE_BUDGET, SCEC_EXCEEDED_LIMIT,
             "return value pushes events size above network config maximum",
             {makeU64SCVal(metrics.mEmitEventByte),
@@ -716,9 +716,9 @@ InvokeHostFunctionOpFrame::doCheckValid(
     if (hostFn.type() == HOST_FUNCTION_TYPE_UPLOAD_CONTRACT_WASM &&
         hostFn.wasm().size() > networkConfig.maxContractSizeBytes())
     {
-        mParentTx.pushSimpleDiagnosticError(
+        mParentTx.pushValidationTimeDiagnosticError(
             appConfig, SCE_BUDGET, SCEC_EXCEEDED_LIMIT,
-            "uploaded WASM size exceeds network config maximum contract size",
+            "uploaded Wasm size exceeds network config maximum contract size",
             {makeU64SCVal(hostFn.wasm().size()),
              makeU64SCVal(networkConfig.maxContractSizeBytes())});
         return false;
@@ -729,7 +729,7 @@ InvokeHostFunctionOpFrame::doCheckValid(
         if (preimage.type() == CONTRACT_ID_PREIMAGE_FROM_ASSET &&
             !isAssetValid(preimage.fromAsset(), ledgerVersion))
         {
-            mParentTx.pushSimpleDiagnosticError(
+            mParentTx.pushValidationTimeDiagnosticError(
                 appConfig, SCE_VALUE, SCEC_INVALID_INPUT,
                 "invalid asset to create contract from");
             return false;

src/transactions/RestoreFootprintOpFrame.cpp

@@ -107,7 +107,7 @@ RestoreFootprintOpFrame::doApply(Application& app, AbstractLedgerTxn& ltx,
         metrics.mLedgerReadByte += entrySize;
         if (resources.readBytes < metrics.mLedgerReadByte)
         {
-            mParentTx.pushSimpleDiagnosticError(
+            mParentTx.pushApplyTimeDiagnosticError(
                 appConfig, SCE_BUDGET, SCEC_EXCEEDED_LIMIT,
                 "operation byte-read resources exceeds amount specified",
                 {makeU64SCVal(metrics.mLedgerReadByte),
@@ -128,7 +128,7 @@ RestoreFootprintOpFrame::doApply(Application& app, AbstractLedgerTxn& ltx,
 
         if (resources.writeBytes < metrics.mLedgerWriteByte)
         {
-            mParentTx.pushSimpleDiagnosticError(
+            mParentTx.pushApplyTimeDiagnosticError(
                 appConfig, SCE_BUDGET, SCEC_EXCEEDED_LIMIT,
                 "operation byte-write resources exceeds amount specified",
                 {makeU64SCVal(metrics.mLedgerWriteByte),
@@ -181,7 +181,7 @@ RestoreFootprintOpFrame::doCheckValid(SorobanNetworkConfig const& networkConfig,
     if (!footprint.readOnly.empty())
     {
         innerResult().code(RESTORE_FOOTPRINT_MALFORMED);
-        mParentTx.pushSimpleDiagnosticError(
+        mParentTx.pushValidationTimeDiagnosticError(
             appConfig, SCE_STORAGE, SCEC_INVALID_INPUT,
             "read-only footprint must be empty for RestoreFootprint operation",
             {});
@@ -193,7 +193,7 @@ RestoreFootprintOpFrame::doCheckValid(SorobanNetworkConfig const& networkConfig,
         if (!isPersistentEntry(lk))
         {
             innerResult().code(RESTORE_FOOTPRINT_MALFORMED);
-            mParentTx.pushSimpleDiagnosticError(
+            mParentTx.pushValidationTimeDiagnosticError(
                 appConfig, SCE_STORAGE, SCEC_INVALID_INPUT,
                 "only persistent Soroban entries can be restored", {});
             return false;

src/transactions/TransactionFrame.cpp

@@ -149,10 +149,6 @@ TransactionFrame::pushSimpleDiagnosticError(Config const& cfg, SCErrorType ty,
                                             std::string&& message,
                                             xdr::xvector<SCVal>&& args)
 {
-    if (!cfg.ENABLE_SOROBAN_DIAGNOSTIC_EVENTS)
-    {
-        return;
-    }
     ContractEvent ce;
     ce.type = DIAGNOSTIC;
     ce.body.v(0);
@@ -182,6 +178,35 @@ TransactionFrame::pushSimpleDiagnosticError(Config const& cfg, SCErrorType ty,
     pushDiagnosticEvent(std::move(evt));
 }
 
+void
+TransactionFrame::pushApplyTimeDiagnosticError(Config const& cfg,
+                                               SCErrorType ty, SCErrorCode code,
+                                               std::string&& message,
+                                               xdr::xvector<SCVal>&& args)
+{
+    if (!cfg.ENABLE_SOROBAN_DIAGNOSTIC_EVENTS)
+    {
+        return;
+    }
+    pushSimpleDiagnosticError(cfg, ty, code, std::move(message),
+                              std::move(args));
+}
+
+void
+TransactionFrame::pushValidationTimeDiagnosticError(Config const& cfg,
+                                                    SCErrorType ty,
+                                                    SCErrorCode code,
+                                                    std::string&& message,
+                                                    xdr::xvector<SCVal>&& args)
+{
+    if (!cfg.ENABLE_DIAGNOSTICS_FOR_TX_SUBMISSION)
+    {
+        return;
+    }
+    pushSimpleDiagnosticError(cfg, ty, code, std::move(message),
+                              std::move(args));
+}
+
 void
 TransactionFrame::setReturnValue(SCVal&& returnValue)
 {
@@ -645,7 +670,7 @@ TransactionFrame::validateSorobanResources(SorobanNetworkConfig const& config,
     auto const& writeEntries = resources.footprint.readWrite;
     if (resources.instructions > config.txMaxInstructions())
     {
-        pushSimpleDiagnosticError(
+        pushValidationTimeDiagnosticError(
             appConfig, SCE_BUDGET, SCEC_EXCEEDED_LIMIT,
             "transaction instructions resources exceed network config limit",
             {makeU64SCVal(resources.instructions),
@@ -654,7 +679,7 @@ TransactionFrame::validateSorobanResources(SorobanNetworkConfig const& config,
     }
     if (resources.readBytes > config.txMaxReadBytes())
     {
-        pushSimpleDiagnosticError(
+        pushValidationTimeDiagnosticError(
             appConfig, SCE_STORAGE, SCEC_EXCEEDED_LIMIT,
             "transaction byte-read resources exceed network config limit",
             {makeU64SCVal(resources.readBytes),
@@ -663,7 +688,7 @@ TransactionFrame::validateSorobanResources(SorobanNetworkConfig const& config,
     }
     if (resources.writeBytes > config.txMaxWriteBytes())
     {
-        pushSimpleDiagnosticError(
+        pushValidationTimeDiagnosticError(
             appConfig, SCE_STORAGE, SCEC_EXCEEDED_LIMIT,
             "transaction byte-write resources exceed network config limit",
             {makeU64SCVal(resources.writeBytes),
@@ -673,7 +698,7 @@ TransactionFrame::validateSorobanResources(SorobanNetworkConfig const& config,
     if (readEntries.size() + writeEntries.size() >
         config.txMaxReadLedgerEntries())
     {
-        pushSimpleDiagnosticError(
+        pushValidationTimeDiagnosticError(
             appConfig, SCE_STORAGE, SCEC_EXCEEDED_LIMIT,
             "transaction entry-read resources exceed network config limit",
             {makeU64SCVal(readEntries.size() + writeEntries.size()),
@@ -682,7 +707,7 @@ TransactionFrame::validateSorobanResources(SorobanNetworkConfig const& config,
     }
     if (writeEntries.size() > config.txMaxWriteLedgerEntries())
     {
-        pushSimpleDiagnosticError(
+        pushValidationTimeDiagnosticError(
             appConfig, SCE_STORAGE, SCEC_EXCEEDED_LIMIT,
             "transaction entry-write resources exceed network config limit",
             {makeU64SCVal(writeEntries.size()),
@@ -703,7 +728,7 @@ TransactionFrame::validateSorobanResources(SorobanNetworkConfig const& config,
                 (tl.asset.type() == ASSET_TYPE_NATIVE) ||
                 isIssuer(tl.accountID, tl.asset))
             {
-                this->pushSimpleDiagnosticError(
+                pushValidationTimeDiagnosticError(
                     appConfig, SCE_STORAGE, SCEC_INVALID_INPUT,
                     "transaction footprint contains invalid trustline asset");
                 return false;
@@ -716,7 +741,7 @@ TransactionFrame::validateSorobanResources(SorobanNetworkConfig const& config,
         case LIQUIDITY_POOL:
         case CONFIG_SETTING:
         case TTL:
-            this->pushSimpleDiagnosticError(
+            pushValidationTimeDiagnosticError(
                 appConfig, SCE_STORAGE, SCEC_UNEXPECTED_TYPE,
                 "transaction footprint contains unsupported ledger key type",
                 {makeU64SCVal(key.type())});
@@ -727,7 +752,7 @@ TransactionFrame::validateSorobanResources(SorobanNetworkConfig const& config,
 
         if (xdr::xdr_size(key) > config.maxContractDataKeySizeBytes())
         {
-            this->pushSimpleDiagnosticError(
+            pushValidationTimeDiagnosticError(
                 appConfig, SCE_STORAGE, SCEC_EXCEEDED_LIMIT,
                 "transaction footprint key exceeds network config limit",
                 {makeU64SCVal(xdr::xdr_size(key)),
@@ -865,7 +890,7 @@ TransactionFrame::consumeRefundableSorobanResources(
     feeRefund = declaredSorobanResourceFee() - preApplyFee.non_refundable_fee;
     if (feeRefund < consumedRentFee)
     {
-        pushSimpleDiagnosticError(
+        pushApplyTimeDiagnosticError(
             cfg, SCE_BUDGET, SCEC_EXCEEDED_LIMIT,
             "refundable resource fee was not sufficient to cover the ledger "
             "storage rent: {} > {}",
@@ -881,7 +906,7 @@ TransactionFrame::consumeRefundableSorobanResources(
         consumedContractEventsSizeBytes, sorobanConfig, cfg);
     if (feeRefund < consumedFee.refundable_fee)
     {
-        pushSimpleDiagnosticError(
+        pushApplyTimeDiagnosticError(
             cfg, SCE_BUDGET, SCEC_EXCEEDED_LIMIT,
             "refundable resource fee was not sufficient to cover the events "
             "fee after paying for ledger storage rent: {} > {}",
@@ -1071,7 +1096,7 @@ TransactionFrame::commonValidPreSeqNum(
         auto const& sorobanData = mEnvelope.v1().tx.ext.sorobanData();
         if (sorobanData.resourceFee > getFullFee())
         {
-            pushSimpleDiagnosticError(
+            pushValidationTimeDiagnosticError(
                 app.getConfig(), SCE_STORAGE, SCEC_EXCEEDED_LIMIT,
                 "transaction `sorobanData.resourceFee` is higher than the "
                 "full transaction fee",
@@ -1084,7 +1109,7 @@ TransactionFrame::commonValidPreSeqNum(
         if (sorobanResourceFee->refundable_fee >
             INT64_MAX - sorobanResourceFee->non_refundable_fee)
         {
-            pushSimpleDiagnosticError(
+            pushValidationTimeDiagnosticError(
                 app.getConfig(), SCE_STORAGE, SCEC_INVALID_INPUT,
                 "transaction resource fees cannot be added",
                 {makeU64SCVal(sorobanResourceFee->refundable_fee),
@@ -1096,7 +1121,7 @@ TransactionFrame::commonValidPreSeqNum(
                                   sorobanResourceFee->non_refundable_fee;
         if (sorobanData.resourceFee < resourceFees)
         {
-            pushSimpleDiagnosticError(
+            pushValidationTimeDiagnosticError(
                 app.getConfig(), SCE_STORAGE, SCEC_EXCEEDED_LIMIT,
                 "transaction `sorobanData.resourceFee` is lower than the "
                 "actual Soroban resource fee",
@@ -1114,7 +1139,7 @@ TransactionFrame::commonValidPreSeqNum(
             {
                 if (!set.emplace(lk).second)
                 {
-                    pushSimpleDiagnosticError(
+                    pushValidationTimeDiagnosticError(
                         app.getConfig(), SCE_STORAGE, SCEC_INVALID_INPUT,
                         "Found duplicate key in the Soroban footprint; every "
                         "key across read-only and read-write footprints has to "