Refuse a module whose output names its fields otherwise while sends wait

Replacing a module, or starting one over a store, kept an output's
waiting sends when the module had the output with as many fields of the
same types. Their names were not compared, and the stored links kept the
types alone: an output whose two numbers swapped places read debit 10,
credit 20 as credit 10, debit 20, replaced and started again (review
004, F3). A send's bytes mean its fields' names and types in order; now
both are compared, in memory and in the store, whose links (PLK4) keep
each field's name. Links kept before (PLK3) have the types alone, as
before; the next save writes PLK4.

test-module's output_meaning: the swapped module is refused, replacing
and starting, while a send waits; trimmed, it replaces and its sends go
on; and the store then refuses the module before.

3 files changed, 86 insertions, 21 deletions

diff --git a/core/module-save.c b/core/module-save.c
index d38437f..e737276 100644
--- a/core/module-save.c
+++ b/core/module-save.c
@@ -42,9 +42,10 @@ unsigned long pure_save_test_checkpoints;
 enum { JOURNAL_MIN = 1 << 20 };
 #define LOG_KEPT (UINT64_C(64) << 20)
 #endif
-/* links: "PLK3"; u8 outputs, each {u8 name length, name, u8 field count, the
- * fields' types, u64le last number} (the module's outputs, then those it
- * dropped, without fields). Before the outbox list, "PLK2" kept the sent
+/* links: "PLK4"; u8 outputs, each {u8 name length, name, u8 field count, each
+ * field's {u8 type, u8 name length, name}, u64le last number} (the module's
+ * outputs, then those it dropped, without fields). "PLK3" kept the fields'
+ * types alone. Before the outbox list, "PLK2" kept the sent
  * messages after them, to the end, each {u8 output (in the table), u64le
  * number, u32le size, payload}; before the heard list, "PLK1" also kept the
  * numbers heard between the two: u16le sources, each {u8 length, the name,
@@ -56,8 +57,8 @@ enum { JOURNAL_MIN = 1 << 20 };
  * seq number, payload bytes). What only grows with the sources a node hears
  * from, or with the sends its receivers have not stored yet, so costs a
  * save only the records of its changes. */
-static const unsigned char links_magic[4] = {'P', 'L', 'K', '3'}, links_magic_2[4] = {'P', 'L', 'K', '2'},
-                           links_magic_1[4] = {'P', 'L', 'K', '1'};
+static const unsigned char links_magic[4] = {'P', 'L', 'K', '4'}, links_magic_3[4] = {'P', 'L', 'K', '3'},
+                           links_magic_2[4] = {'P', 'L', 'K', '2'}, links_magic_1[4] = {'P', 'L', 'K', '1'};
 typedef struct { const char *name; unsigned fields; uint8_t kinds[3]; char names[3][PURE_MODULE_NAME]; } RuntimeList;
 static const RuntimeList runtime_lists[PURE_LISTS_STORED - PURE_LIST_HEARD] = {
     {"0heard", 2, {1, 0}, {"source", "seq"}},
@@ -644,7 +645,11 @@ PURE_COLD PureResult pure_save_open_values(PureApp *app, const PureModule *m, do
 
 static bool same_version(PureVersion a, PureVersion b) { return a.ref == b.ref && a.bytes == b.bytes; }
 
-/* The links as stored; NULL when memory runs out. */
+/* The links as stored; NULL when memory runs out. Each output: its name,
+ * its fields (each its type and name: what its sends' bytes mean, so a
+ * module that orders or names them otherwise is refused while sends wait),
+ * and the last number it sent; then the outputs carried from modules
+ * before, with none. */
 static unsigned char *encode_links(const PureApp *app, size_t *size) {
     const PureModule *m = app->module;
     unsigned outputs = 0;
@@ -653,7 +658,9 @@ static unsigned char *encode_links(const PureApp *app, size_t *size) {
     for (uint32_t i = 0; i < m->inputs; ++i)
         if (m->input[i].kind == PURE_KIND_OUTPUT) {
             table[i] = (uint8_t)outputs++;
-            total += 2 + strlen(m->input_names[i]) + m->input[i].field_count + 8;
+            total += 2 + strlen(m->input_names[i]) + 8;
+            for (uint32_t f = 0; f < m->input[i].field_count; ++f)
+                total += 2 + strlen(m->field[m->input[i].field_start + f].name);
         }
     for (unsigned c = 0; c < app->carried_count; ++c) total += 2 + strlen(app->carried[c].name) + 8;
     unsigned char *bytes = malloc(total), *at = bytes;
@@ -668,7 +675,14 @@ static unsigned char *encode_links(const PureApp *app, size_t *size) {
         memcpy(at, m->input_names[i], length);
         at += length;
         *at++ = (unsigned char)m->input[i].field_count;
-        for (uint32_t f = 0; f < m->input[i].field_count; ++f) *at++ = m->field[m->input[i].field_start + f].type;
+        for (uint32_t f = 0; f < m->input[i].field_count; ++f) {
+            const PureField *field = &m->field[m->input[i].field_start + f];
+            size_t named = strlen(field->name);
+            *at++ = field->type;
+            *at++ = (unsigned char)named;
+            memcpy(at, field->name, named);
+            at += named;
+        }
         at = put_le(at, app->last_sent[i], 8);
     }
     for (unsigned c = 0; c < app->carried_count; ++c) {
@@ -706,7 +720,8 @@ PURE_COLD static PureResult sent_before(PureApp *app, const char *output, uint64
  * fields changed or that the module lacks refuses the start while it has
  * sends left in the outbox: CONFLICT; one without sends keeps its number
  * for later); the sources and sends links kept before the runtime's lists
- * go to them (heard_moved, sent_moved: they need images). */
+ * go to them (heard_moved, sent_moved: they need images). Links kept
+ * before PLK4 have their fields' types alone. */
 PURE_COLD PureResult pure_save_open_links(PureApp *app, const PureModule *m, bool *heard_moved, bool *sent_moved) {
     *heard_moved = *sent_moved = false;
     PureResult result = app->stored_links ? PURE_OK : open_state(app, "", "links", &app->stored_links);
@@ -719,7 +734,8 @@ PURE_COLD PureResult pure_save_open_links(PureApp *app, const PureModule *m, boo
     char names[256][PURE_MODULE_NAME];
     size_t at = 5;
     bool first = size >= 5 && !memcmp(bytes, links_magic_1, 4), second = size >= 5 && !memcmp(bytes, links_magic_2, 4),
-         known = first || second || (size >= 5 && !memcmp(bytes, links_magic, 4));
+         named = size >= 5 && !memcmp(bytes, links_magic, 4),
+         known = first || second || named || (size >= 5 && !memcmp(bytes, links_magic_3, 4));
     unsigned outputs = known ? bytes[4] : 0;
     result = known ? PURE_OK : PURE_STORE_ERROR;
     for (unsigned t = 0; t < outputs && result == PURE_OK; ++t) {
@@ -730,13 +746,20 @@ PURE_COLD PureResult pure_save_open_links(PureApp *app, const PureModule *m, boo
         name[length] = 0;
         unsigned fields = bytes[at + 1 + length];
         at += 2 + length;
-        if (size - at < fields + 8) { result = PURE_STORE_ERROR; break; }
         int i = pure_module_input(m, name);
         output[t] = i >= 0 && m->input[i].kind == PURE_KIND_OUTPUT ? i : -1;
         fits[t] = output[t] >= 0 && m->input[i].field_count == fields;
-        for (unsigned f = 0; fits[t] && f < fields; ++f) fits[t] = m->field[m->input[i].field_start + f].type == bytes[at + f];
-        uint64_t last = get_le(bytes + at + fields, 8);
-        at += fields + 8;
+        for (unsigned f = 0; f < fields && result == PURE_OK; ++f) { /* its type, then (PLK4) its name */
+            size_t named_length = named && size - at >= 2 ? bytes[at + 1] : 0;
+            if (size - at < (named ? 2 + named_length : 1)) { result = PURE_STORE_ERROR; break; }
+            const PureField *field = fits[t] ? &m->field[m->input[i].field_start + f] : NULL;
+            fits[t] = field && field->type == bytes[at] &&
+                      (!named || (strlen(field->name) == named_length && !memcmp(field->name, bytes + at + 2, named_length)));
+            at += named ? 2 + named_length : 1;
+        }
+        if (result != PURE_OK || size - at < 8) { result = PURE_STORE_ERROR; break; }
+        uint64_t last = get_le(bytes + at, 8);
+        at += 8;
         if (output[t] >= 0) app->last_sent[output[t]] = last;
         else if (last) {
             struct PureCarried *grown = realloc(app->carried, (app->carried_count + 1) * sizeof(*grown));
diff --git a/core/module.c b/core/module.c
index 574d6af..c65ef80 100644
--- a/core/module.c
+++ b/core/module.c
@@ -3246,9 +3246,10 @@ PURE_COLD static PureResult keep_list(PureList **out, PureList *from, const Pure
  * again, and the next event or save publishes the module's states; a state
  * the module drops stays stored as it was. Starting an app opens its module
  * over none. */
-/* Outputs of old that module has by name and with the same fields keep their
- * sends and numbers; one it lacks or changed refuses the module while sends
- * are left on it (CONFLICT), and otherwise keeps its number for later. */
+/* Outputs of old that module has by name and with the same fields (their
+ * names and types, in order: what a send's bytes mean) keep their sends and
+ * numbers; one it lacks or changed refuses the module while sends are left
+ * on it (CONFLICT), and otherwise keeps its number for later. */
 PURE_COLD static PureResult remap_outputs(PureApp *app, const PureModule *old, const PureModule *module, bool apply) {
     int map[PURE_MODULE_INPUTS];
     for (uint32_t i = 0; i < old->inputs; ++i) {
@@ -3257,8 +3258,11 @@ PURE_COLD static PureResult remap_outputs(PureApp *app, const PureModule *old, c
         int to = pure_module_input(module, old->input_names[i]);
         bool same = to >= 0 && module->input[to].kind == PURE_KIND_OUTPUT &&
                     module->input[to].field_count == old->input[i].field_count;
-        for (uint32_t f = 0; same && f < old->input[i].field_count; ++f)
-            same = module->field[module->input[to].field_start + f].type == old->field[old->input[i].field_start + f].type;
+        for (uint32_t f = 0; same && f < old->input[i].field_count; ++f) {
+            const PureField *now = &module->field[module->input[to].field_start + f],
+                            *was = &old->field[old->input[i].field_start + f];
+            same = now->type == was->type && !strcmp(now->name, was->name);
+        }
         map[i] = same ? to : -1;
         size_t length = strlen(old->input_names[i]);
         for (size_t s = 0, count = app->outbox ? pure_list_count(app->outbox) : 0; !same && s < count; ++s) {
diff --git a/core/test-module.c b/core/test-module.c
index 959e72e..788bfc6 100644
--- a/core/test-module.c
+++ b/core/test-module.c
@@ -3997,7 +3997,7 @@ static void links(void) {
     CHECK(pure_app_heard(&b, "legacy") == 5 && pure_app_heard(&b, "orders.ordered") == 2 && pure_app_heard(&b, "x") == 1);
     CHECK(deliver(&b, "legacy", 5, "old", 1) == PURE_OK && b.duplicate && deliver(&b, "legacy", 6, "new", 1) == PURE_OK && !b.duplicate);
     pure_app_stop(&b);
-    CHECK(pure_state_open(path_b, "links", &stored_links) == PURE_OK && !memcmp(read_state(stored_links), "PLK3", 4));
+    CHECK(pure_state_open(path_b, "links", &stored_links) == PURE_OK && !memcmp(read_state(stored_links), "PLK4", 4));
     pure_draft_close(stored_links);
     /* Links stored before the outbox list kept the sends: a start moves them
      * to it; numbering goes on from the output's last number. */
@@ -4029,7 +4029,7 @@ static void links(void) {
           !memcmp(o.items[1].payload, "abcd", 4));
     CHECK(buy(&c, 0, "next", 1) == PURE_OK && outbox_of(&c).items[2].seq == 3);
     pure_app_stop(&c);
-    CHECK(pure_state_open(path_c, "links", &stored_links) == PURE_OK && !memcmp(read_state(stored_links), "PLK3", 4));
+    CHECK(pure_state_open(path_c, "links", &stored_links) == PURE_OK && !memcmp(read_state(stored_links), "PLK4", 4));
     pure_draft_close(stored_links);
     /* A large outbox costs a save only its changes: the links stay small, the
      * sends and trims come back after a start. */
@@ -4158,6 +4158,43 @@ static void links(void) {
     CHECK(load(bad, &module_b) == PURE_INVALID && strstr(error, "handler event"));
 }
 
+/* An output whose fields are named otherwise (here two numbers swapped) is
+ * another output: its waiting sends' bytes would mean something else. A
+ * module that has it so is refused while sends wait, replacing or starting
+ * over the store; once they are trimmed it replaces (review 004, F3). */
+static void output_meaning(void) {
+    static const S nums[] = {{"n", 0}};
+    static const F was_fields[] = {{"debit", PURE_FIELD_NUMBER}, {"credit", PURE_FIELD_NUMBER}};
+    static const F now_fields[] = {{"credit", PURE_FIELD_NUMBER}, {"debit", PURE_FIELD_NUMBER}};
+    static const I was_inputs[] = {KEY("tap", "KeyT"), {"pay", NULL, PURE_KIND_OUTPUT, false, was_fields, 2}};
+    static const I now_inputs[] = {KEY("tap", "KeyT"), {"pay", NULL, PURE_KIND_OUTPUT, false, now_fields, 2}};
+    static const O was_code[] = {{C, 0, 10}, {C, 0, 20}, {SEND, 1, 0}}; /* pay { debit: 10, credit: 20 } */
+    static const O now_code[] = {{C, 0, 20}, {C, 0, 10}, {SEND, 1, 0}}; /* the same, fields in the new order */
+    static const H handlers[] = {{1, 0, 3}};
+    Spec was = {nums, 1, was_inputs, 2, NULL, 0, handlers, 1, was_code, 3, NULL, 0, NULL, 0, NULL, 0, NULL, 0, NULL, 0,
+                NULL, 0};
+    Spec now = {nums, 1, now_inputs, 2, NULL, 0, handlers, 1, now_code, 3, NULL, 0, NULL, 0, NULL, 0, NULL, 0, NULL, 0,
+                NULL, 0};
+    CHECK(load(was, &module_a) == PURE_OK && load(now, &module_b) == PURE_OK);
+    CHECK(pure_module_shape(&module_a, 1) != pure_module_shape(&module_b, 1));
+    char *path = new_store();
+    PureApp app;
+    CHECK(pure_app_start(&app, &module_a, pure_tree_data(path)) == PURE_OK);
+    CHECK(pure_app_input(&app, 0, PURE_EDGE_DOWN) == PURE_OK && outbox_of(&app).count == 1);
+    CHECK(pure_app_replace(&app, &module_b) == PURE_CONFLICT && app.module == &module_a);
+    pure_app_stop(&app);
+    CHECK(pure_app_start(&app, &module_b, pure_tree_data(path)) == PURE_CONFLICT);
+    CHECK(pure_app_start(&app, &module_a, pure_tree_data(path)) == PURE_OK && outbox_of(&app).count == 1);
+    pure_app_trim(&app, 1, 1);
+    CHECK(pure_app_save(&app) == PURE_OK && pure_app_replace(&app, &module_b) == PURE_OK);
+    CHECK(pure_app_input(&app, 0, PURE_EDGE_DOWN) == PURE_OK && outbox_of(&app).items[0].seq == 2);
+    pure_app_stop(&app);
+    CHECK(pure_app_start(&app, &module_b, pure_tree_data(path)) == PURE_OK && outbox_of(&app).count == 1);
+    pure_app_stop(&app);
+    CHECK(pure_app_start(&app, &module_a, pure_tree_data(path)) == PURE_CONFLICT);
+    remove_store(path);
+}
+
 /* A timer's tick sees the time the host set; floor and a floored modulo
  * take it apart. now is for handlers only. */
 enum { NOW = PURE_OP_NOW, FLOOR = PURE_OP_FLOOR, MOD = PURE_OP_MOD };
@@ -4271,6 +4308,7 @@ int main(void) {
     keyed_mutations();
     snapshots();
     links();
+    output_meaning();
     fold_mutations();
     pointing();
     list_mutations();