idempotent event deletion, prevent levId reuse when latest event is replaced or deleted

src/RelayCron.cpp

@@ -57,10 +57,7 @@ void RelayServer::runCron() {
             uint64_t numDeleted = 0;
 
             for (auto levId : expiredLevIds) {
-                auto view = env.lookup_Event(txn, levId);
+                if (deleteEvent(txn, levId)) numDeleted++;
-                if (!view) continue; // Deleted in between transactions
-                deleteEvent(txn, *view);
-                numDeleted++;
             }
 
             txn.commit();
@@ -114,10 +111,7 @@ void RelayServer::runCron() {
             uint64_t numDeleted = 0;
 
             for (auto levId : expiredLevIds) {
-                auto view = env.lookup_Event(txn, levId);
+                if (deleteEvent(txn, levId)) numDeleted++;
-                if (!view) continue; // Deleted in between transactions
-                deleteEvent(txn, *view);
-                numDeleted++;
             }
 
             txn.commit();

src/cmd_delete.cpp

@@ -70,9 +70,7 @@ void cmd_delete(const std::vector<std::string> &subArgs) {
         auto txn = env.txn_rw();
 
         for (auto levId : levIds) {
-            auto view = env.lookup_Event(txn, levId);
+            deleteEvent(txn, levId);
-            if (!view) continue; // Deleted in between transactions
-            deleteEvent(txn, *view);
         }
 
         txn.commit();

src/events.cpp

@@ -243,9 +243,10 @@ std::string_view getEventJson(lmdb::txn &txn, Decompressor &decomp, uint64_t lev
 
 
 
-void deleteEvent(lmdb::txn &txn, defaultDb::environment::View_Event &ev) {
+bool deleteEvent(lmdb::txn &txn, uint64_t levId) {
-    env.dbi_EventPayload.del(txn, lmdb::to_sv<uint64_t>(ev.primaryKeyId));
+    bool deleted = env.dbi_EventPayload.del(txn, lmdb::to_sv<uint64_t>(levId));
-    env.delete_Event(txn, ev.primaryKeyId);
+    env.delete_Event(txn, levId);
+    return deleted;
 }
 
 
@@ -258,6 +259,7 @@ void writeEvents(lmdb::txn &txn, std::vector<EventToWrite> &evs, uint64_t logLev
         return aC < bC;
     });
 
+    std::vector<uint64_t> levIdsToDelete;
     std::string tmpBuf;
 
     for (size_t i = 0; i < evs.size(); i++) {
@@ -296,7 +298,7 @@ void writeEvents(lmdb::txn &txn, std::vector<EventToWrite> &evs, uint64_t logLev
 
                         if (otherEv.flat_nested()->created_at() < flat->created_at()) {
                             if (logLevel >= 1) LI << "Deleting event (d-tag). id=" << to_hex(sv(otherEv.flat_nested()->id()));
-                            deleteEvent(txn, otherEv);
+                            levIdsToDelete.push_back(otherEv.primaryKeyId);
                         } else {
                             ev.status = EventWriteStatus::Replaced;
                         }
@@ -314,7 +316,7 @@ void writeEvents(lmdb::txn &txn, std::vector<EventToWrite> &evs, uint64_t logLev
                     auto otherEv = lookupEventById(txn, sv(tagPair->val()));
                     if (otherEv && sv(otherEv->flat_nested()->pubkey()) == sv(flat->pubkey())) {
                         if (logLevel >= 1) LI << "Deleting event (kind 5). id=" << to_hex(sv(tagPair->val()));
-                        deleteEvent(txn, *otherEv);
+                        levIdsToDelete.push_back(otherEv->primaryKeyId);
                     }
                 }
             }
@@ -329,6 +331,13 @@ void writeEvents(lmdb::txn &txn, std::vector<EventToWrite> &evs, uint64_t logLev
             env.dbi_EventPayload.put(txn, lmdb::to_sv<uint64_t>(ev.levId), tmpBuf);
 
             ev.status = EventWriteStatus::Written;
+
+            // Deletions happen after event was written to ensure levIds are not reused
+
+            for (auto levId : levIdsToDelete) deleteEvent(txn, levId);
+            levIdsToDelete.clear();
         }
+
+        if (levIdsToDelete.size()) throw herr("unprocessed deletion");
     }
 }

src/events.h

@@ -112,4 +112,4 @@ struct EventToWrite {
 
 
 void writeEvents(lmdb::txn &txn, std::vector<EventToWrite> &evs, uint64_t logLevel = 1);
-void deleteEvent(lmdb::txn &txn, defaultDb::environment::View_Event &ev);
+bool deleteEvent(lmdb::txn &txn, uint64_t levId);

test/writeTest.pl

@@ -22,9 +22,8 @@ my $ids = [
 
 
 
-## Basic insert
-
 doTest({
+    desc => "Basic insert",
     events => [
         qq{--sec $ids->[0]->{sec} --content "hi" --kind 1 },
         qq{--sec $ids->[0]->{sec} --content "hi 2" --kind 1 },
@@ -32,9 +31,9 @@ doTest({
     verify => [ 0, 1, ],
 });
 
-## Replacement, newer timestamp
 
 doTest({
+    desc => "Replacement, newer timestamp",
     events => [
         qq{--sec $ids->[0]->{sec} --content "hi" --kind 10000 --created-at 5000 },
         qq{--sec $ids->[0]->{sec} --content "hi 2" --kind 10000 --created-at 5001 },
@@ -43,9 +42,9 @@ doTest({
     verify => [ 1, ],
 });
 
-## Same, but explicit empty d tag
 
 doTest({
+    desc => "Same, but explicit empty d tag",
     events => [
         qq{--sec $ids->[0]->{sec} --content "hi" --kind 10000 --created-at 5000 },
         qq{--sec $ids->[0]->{sec} --content "hi 2" --kind 10000 --created-at 5001 --tag d '' },
@@ -54,9 +53,9 @@ doTest({
     verify => [ 1, ],
 });
 
-## Replacement is dropped
 
 doTest({
+    desc => "Replacement is dropped",
     events => [
         qq{--sec $ids->[0]->{sec} --content "hi" --kind 10000 --created-at 5001 },
         qq{--sec $ids->[0]->{sec} --content "hi 2" --kind 10000 --created-at 5000 },
@@ -64,9 +63,9 @@ doTest({
     verify => [ 0, ],
 });
 
-## Doesn't replace some else's event
 
 doTest({
+    desc => "Doesn't replace some else's event",
     events => [
         qq{--sec $ids->[0]->{sec} --content "hi" --kind 10000 --created-at 5000 },
         qq{--sec $ids->[1]->{sec} --content "hi 2" --kind 10000 --created-at 5001 },
@@ -74,9 +73,9 @@ doTest({
     verify => [ 0, 1, ],
 });
 
-## Doesn't replace different kind
 
 doTest({
+    desc => "Doesn't replace different kind",
     events => [
         qq{--sec $ids->[0]->{sec} --content "hi" --kind 10001 --created-at 5000 },
         qq{--sec $ids->[1]->{sec} --content "hi 2" --kind 10000 --created-at 5001 },
@@ -85,9 +84,9 @@ doTest({
 });
 
 
-## Deletion
 
 doTest({
+    desc => "Deletion",
     events => [
         qq{--sec $ids->[0]->{sec} --content "hi" --kind 1 --created-at 5000 },
         qq{--sec $ids->[0]->{sec} --content "hi" --kind 1 --created-at 5001 },
@@ -97,9 +96,21 @@ doTest({
     verify => [ 1, 3, ],
 });
 
-## Can't delete someone else's event
 
 doTest({
+    desc => "Deletion, duplicate",
+    events => [
+        qq{--sec $ids->[0]->{sec} --content "hi" --kind 1 --created-at 5000 },
+        qq{--sec $ids->[0]->{sec} --content "hi" --kind 1 --created-at 5001 },
+        qq{--sec $ids->[0]->{sec} --content "hi" --kind 1 --created-at 5002 },
+        qq{--sec $ids->[0]->{sec} --content "blah" --kind 5 --created-at 6000 -e EV_2 -e EV_2 },
+    ],
+    verify => [ 0, 1, 3, ],
+});
+
+
+doTest({
+    desc => "Can't delete someone else's event",
     events => [
         qq{--sec $ids->[0]->{sec} --content "hi" --kind 1 --created-at 5000 },
         qq{--sec $ids->[1]->{sec} --content "blah" --kind 5 --created-at 6000 -e EV_0 },
@@ -107,9 +118,9 @@ doTest({
     verify => [ 0, 1, ],
 });
 
-## Deletion prevents re-adding same event
 
 doTest({
+    desc => "Deletion prevents re-adding same event",
     events => [
         qq{--sec $ids->[0]->{sec} --content "hi" --kind 1 --created-at 5000 },
         qq{--sec $ids->[0]->{sec} --content "blah" --kind 5 --created-at 6000 -e EV_0 },
@@ -120,9 +131,9 @@ doTest({
 
 
 
-## Parameterized Replaceable Events
 
 doTest({
+    desc => "Parameterized Replaceable Events",
     events => [
         qq{--sec $ids->[0]->{sec} --content "hi1" --kind 1 --created-at 5000 --tag d myrepl },
         qq{--sec $ids->[0]->{sec} --content "hi2" --kind 1 --created-at 5001 --tag d myrepl },
@@ -130,9 +141,9 @@ doTest({
     verify => [ 1, ],
 });
 
-## d tags have to match
 
 doTest({
+    desc => "d tags have to match",
     events => [
         qq{--sec $ids->[0]->{sec} --content "hi1" --kind 1 --created-at 5000 --tag d myrepl },
         qq{--sec $ids->[0]->{sec} --content "hi2" --kind 1 --created-at 5001 --tag d myrepl2 },
@@ -141,9 +152,9 @@ doTest({
     verify => [ 1, 2, ],
 });
 
-## Kinds have to match
 
 doTest({
+    desc => "Kinds have to match",
     events => [
         qq{--sec $ids->[0]->{sec} --content "hi1" --kind 1 --created-at 5000 --tag d myrepl },
         qq{--sec $ids->[0]->{sec} --content "hi2" --kind 2 --created-at 5001 --tag d myrepl },
@@ -151,9 +162,9 @@ doTest({
     verify => [ 0, 1, ],
 });
 
-## Pubkeys have to match
 
 doTest({
+    desc => "Pubkeys have to match",
     events => [
         qq{--sec $ids->[0]->{sec} --content "hi1" --kind 1 --created-at 5000 --tag d myrepl },
         qq{--sec $ids->[1]->{sec} --content "hi2" --kind 1 --created-at 5001 --tag d myrepl },
@@ -161,9 +172,9 @@ doTest({
     verify => [ 0, 1, ],
 });
 
-## Timestamp
 
 doTest({
+    desc => "Timestamp",
     events => [
         qq{--sec $ids->[0]->{sec} --content "hi1" --kind 1 --created-at 5001 --tag d myrepl },
         qq{--sec $ids->[0]->{sec} --content "hi2" --kind 1 --created-at 5000 --tag d myrepl },
@@ -173,12 +184,14 @@ doTest({
 
 
 
-print "OK\n";
+print "\nOK\n";
 
 
 sub doTest {
     my $spec = shift;
 
+    print "* ", ($spec->{desc} || 'unnamed'), "\n";
+
     cleanDb();
 
     my $eventIds = [];