refactor(core,accountsdb,bincode,net): Fixes, improvements, renames +…

… discuss (#315)

* Refactor `NestedHashTree.getValue`

* Use term 'zeroes' instead of 'default' in Pubkey

* fuzz: Make TrackedAccount data fixed size
It was always already just 32 bytes, there was no real reason for
it to be dynamically allocated.

* misc:
 * bincode: Improved names and functionality + fix
 * fix `bincode.free` for various types
 * renames & unifies `arraylist.defaultArrayList[Unmanaged]OnEOFConfig`
   to `arraylist.defaultOnEofConfig`.
 * renames `arraylist.arrayListFieldConfig` to
   `arraylist.standardConfig`.
 * renames `shortvec.ShortVecConfig` to `shortvec.sliceConfig`.
 * renames `shortvec.ShortVecArrayListConfig`
   to `shortvec.arrayListConfig`.

* Constify data which doesn't need to be mutable

* Fix leaks in error path

* Use init constants instead of functions

* Inline test data, improve std.testing usage

* Further simplify `NestedHashTree`

* Upper-case init decls

* `Hash.default()` -> `Hash.ZEROES`

* Upper-case `CompiledKeyMeta.ALL_FALSE`

* Upper-case `SocketAddr.UNSPECIFIED`

* `ClientVersion.default` -> `ClientVersion.CURRENT`

* `Transaction.empty` -> `Transaction.EMPTY`

* Remove test
It doesn't test anything very useful

* comptime-assert error branch as unreachable

* Inline more test data & use more inference

* remove `socket_addrs_unspecified`

* `isAllZeroes` -> `isZeroed`

src/accountsdb/accounts_file.zig

@@ -154,7 +154,7 @@ pub const AccountInFile = struct {
         const valid_lamports = self.account_info.lamports != 0 or (
         // ie, is default account
             self.data.len == 0 and
-            self.owner().isDefault() and
+            self.owner().isZeroed() and
             self.executable().* == false and
             self.rent_epoch().* == 0);
         if (!valid_lamports) {

src/accountsdb/db.zig

@@ -1035,7 +1035,7 @@ pub const AccountsDB = struct {
                     }
                 } else {
                     // hashes arent always stored correctly in snapshots
-                    if (account_hash.order(&Hash.default()) == .eq) {
+                    if (account_hash.order(&Hash.ZEROES) == .eq) {
                         const account, var lock_guard = try self.getAccountFromRefWithReadLock(max_slot_ref);
                         defer lock_guard.unlock();
 
@@ -2372,7 +2372,7 @@ pub const AccountsDB = struct {
         });
 
         // TODO: this is a temporary value
-        const delta_hash = Hash.default();
+        const delta_hash = Hash.ZEROES;
 
         const archive_file = blk: {
             const archive_file_name_bounded = sig.accounts_db.snapshots.FullSnapshotFileInfo.snapshotNameStr(.{
@@ -2550,7 +2550,7 @@ pub const AccountsDB = struct {
         });
 
         // TODO: compute the correct value during account writes
-        const delta_hash = Hash.default();
+        const delta_hash = Hash.ZEROES;
 
         const archive_file = blk: {
             const archive_file_name_bounded = sig.accounts_db.snapshots.IncrementalSnapshotFileInfo.snapshotNameStr(.{
@@ -2618,7 +2618,7 @@ pub const AccountsDB = struct {
                 .slot = params.target_slot,
                 .bank_hash_info = .{
                     .accounts_delta_hash = delta_hash,
-                    .accounts_hash = Hash.default(),
+                    .accounts_hash = Hash.ZEROES,
                     .stats = bank_hash_stats,
                 },
                 .rooted_slots = .{},
@@ -3258,7 +3258,7 @@ test "write and read an account" {
         .data = &data,
         .executable = false,
         .lamports = 100,
-        .owner = Pubkey.default(),
+        .owner = Pubkey.ZEROES,
         .rent_epoch = 0,
     };
 

src/accountsdb/fuzz.zig

@@ -13,25 +13,23 @@ const BankFields = sig.accounts_db.snapshots.BankFields;
 pub const TrackedAccount = struct {
     pubkey: Pubkey,
     slot: u64,
-    data: []u8,
+    data: [32]u8,
 
-    pub fn initRandom(random: std.rand.Random, slot: Slot, allocator: std.mem.Allocator) !TrackedAccount {
+    pub fn initRandom(random: std.rand.Random, slot: Slot) !TrackedAccount {
+        var data: [32]u8 = undefined;
+        random.bytes(&data);
         return .{
             .pubkey = Pubkey.initRandom(random),
             .slot = slot,
-            .data = try allocator.alloc(u8, 32),
+            .data = data,
         };
     }
 
-    pub fn deinit(self: *TrackedAccount, allocator: std.mem.Allocator) void {
-        allocator.free(self.data);
-    }
-
     pub fn toAccount(self: *const TrackedAccount, allocator: std.mem.Allocator) !Account {
         return .{
             .lamports = 19,
-            .data = try allocator.dupe(u8, self.data),
-            .owner = Pubkey.default(),
+            .data = try allocator.dupe(u8, &self.data),
+            .owner = Pubkey.ZEROES,
             .executable = false,
             .rent_epoch = 0,
         };
@@ -123,9 +121,6 @@ pub fn run(seed: u64, args: *std.process.ArgIterator) !void {
 
     var tracked_accounts = std.AutoArrayHashMap(Pubkey, TrackedAccount).init(allocator);
     defer tracked_accounts.deinit();
-    defer for (tracked_accounts.values()) |*value| {
-        value.deinit(allocator);
-    };
     try tracked_accounts.ensureTotalCapacity(10_000);
 
     var random_bank_fields = try BankFields.initRandom(allocator, random, 1 << 8);
@@ -158,7 +153,7 @@ pub fn run(seed: u64, args: *std.process.ArgIterator) !void {
                 for (&accounts, &pubkeys, 0..) |*account, *pubkey, i| {
                     errdefer for (accounts[0..i]) |prev_account| prev_account.deinit(allocator);
 
-                    var tracked_account = try TrackedAccount.initRandom(random, slot, allocator);
+                    var tracked_account = try TrackedAccount.initRandom(random, slot);
 
                     const existing_pubkey = random.boolean();
                     if (existing_pubkey and tracked_accounts.count() > 0) {
@@ -170,12 +165,8 @@ pub fn run(seed: u64, args: *std.process.ArgIterator) !void {
                     account.* = try tracked_account.toAccount(allocator);
                     pubkey.* = tracked_account.pubkey;
 
-                    const r = try tracked_accounts.getOrPut(tracked_account.pubkey);
-                    if (r.found_existing) {
-                        r.value_ptr.deinit(allocator);
-                    }
                     // always overwrite the old slot
-                    r.value_ptr.* = tracked_account;
+                    try tracked_accounts.put(tracked_account.pubkey, tracked_account);
                 }
                 defer for (accounts) |account| account.deinit(allocator);
 
@@ -198,7 +189,7 @@ pub fn run(seed: u64, args: *std.process.ArgIterator) !void {
                 var account = try accounts_db.getAccount(&tracked_account.pubkey);
                 defer account.deinit(allocator);
 
-                if (!std.mem.eql(u8, tracked_account.data, account.data)) {
+                if (!std.mem.eql(u8, &tracked_account.data, account.data)) {
                     @panic("found accounts with different data");
                 }
             },

src/accountsdb/index.zig

@@ -84,7 +84,7 @@ pub const AccountRef = struct {
 
     pub fn default() AccountRef {
         return AccountRef{
-            .pubkey = Pubkey.default(),
+            .pubkey = Pubkey.ZEROES,
             .slot = 0,
             .location = .{ .UnrootedMap = .{ .index = 0 } },
         };

src/accountsdb/snapshots.zig

@@ -23,7 +23,6 @@ const SlotHistory = sig.accounts_db.sysvars.SlotHistory;
 
 const Logger = sig.trace.Logger;
 
-const defaultArrayListUnmanagedOnEOFConfig = bincode.arraylist.defaultArrayListUnmanagedOnEOFConfig;
 const parallelUntarToFileSystem = sig.utils.tar.parallelUntarToFileSystem;
 const readDirectory = sig.utils.directory.readDirectory;
 
@@ -675,9 +674,9 @@ pub const BankIncrementalSnapshotPersistence = struct {
     pub fn default() @This() {
         return .{
             .full_slot = 0,
-            .full_hash = Hash.default(),
+            .full_hash = Hash.ZEROES,
             .full_capitalization = 0,
-            .incremental_hash = Hash.default(),
+            .incremental_hash = Hash.ZEROES,
             .incremental_capitalization = 0,
         };
     }
@@ -1066,8 +1065,8 @@ pub const AccountsDbFields = struct {
     pub const @"!bincode-config:file_map" = bincode.hashmap.hashMapFieldConfig(FileMap, .{
         .value = bincode.list.valueEncodedAsSlice(AccountFileInfo, .{}),
     });
-    pub const @"!bincode-config:rooted_slots" = defaultArrayListUnmanagedOnEOFConfig(Slot);
-    pub const @"!bincode-config:rooted_slot_hashes" = defaultArrayListUnmanagedOnEOFConfig(SlotAndHash);
+    pub const @"!bincode-config:rooted_slots" = bincode.arraylist.defaultOnEofConfig(std.ArrayListUnmanaged(Slot));
+    pub const @"!bincode-config:rooted_slot_hashes" = bincode.arraylist.defaultOnEofConfig(std.ArrayListUnmanaged(SlotAndHash));
 
     pub const FileMap = std.AutoArrayHashMap(Slot, AccountFileInfo);
 
@@ -1599,7 +1598,7 @@ pub const FullSnapshotFileInfo = struct {
     test snapshotNameStr {
         try std.testing.expectEqualStrings(
             "snapshot-10-11111111111111111111111111111111.tar.zst",
-            snapshotNameStr(.{ .slot = 10, .hash = Hash.default() }).constSlice(),
+            snapshotNameStr(.{ .slot = 10, .hash = Hash.ZEROES }).constSlice(),
         );
     }
 };
@@ -1669,7 +1668,7 @@ pub const IncrementalSnapshotFileInfo = struct {
     test snapshotNameStr {
         try std.testing.expectEqualStrings(
             "incremental-snapshot-10-25-11111111111111111111111111111111.tar.zst",
-            snapshotNameStr(.{ .base_slot = 10, .slot = 25, .hash = Hash.default() }).constSlice(),
+            snapshotNameStr(.{ .base_slot = 10, .slot = 25, .hash = Hash.ZEROES }).constSlice(),
         );
     }
 };

src/accountsdb/swiss_map.zig

@@ -504,11 +504,11 @@ test "swissmap resize" {
     try map.ensureTotalCapacity(100);
 
     const ref = accounts_db.index.AccountRef.default();
-    map.putAssumeCapacity(sig.core.Pubkey.default(), ref);
+    map.putAssumeCapacity(sig.core.Pubkey.ZEROES, ref);
 
     // this will resize the map with the key still in there
     try map.ensureTotalCapacity(200);
-    const get_ref = map.get(sig.core.Pubkey.default()) orelse return error.MissingAccount;
+    const get_ref = map.get(sig.core.Pubkey.ZEROES) orelse return error.MissingAccount;
     try std.testing.expect(std.meta.eql(get_ref, ref));
 }
 

src/bincode/arraylist.zig

@@ -9,8 +9,8 @@ const Params = bincode.Params;
 const readIntAsLength = bincode.readIntAsLength;
 
 /// The standard bincode serialization for an ArrayList
-pub fn arrayListFieldConfig(comptime ArrayListType: type) bincode.FieldConfig(ArrayListType) {
-    const list_info = arrayListInfo(ArrayListType).?;
+pub fn standardConfig(comptime List: type) bincode.FieldConfig(List) {
+    const list_info = arrayListInfo(List).?;
 
     const S = struct {
         fn serialize(writer: anytype, data: anytype, params: bincode.Params) anyerror!void {
@@ -24,10 +24,10 @@ pub fn arrayListFieldConfig(comptime ArrayListType: type) bincode.FieldConfig(Ar
             allocator: std.mem.Allocator,
             reader: anytype,
             params: Params,
-        ) anyerror!ArrayListType {
+        ) anyerror!List {
             const len = (try readIntAsLength(usize, reader, params)) orelse return error.ArrayListTooBig;
 
-            var data: ArrayListType = try ArrayListType.initCapacity(allocator, len);
+            var data: List = try List.initCapacity(allocator, len);
             errdefer bincode.free(allocator, data);
             for (0..len) |_| {
                 data.appendAssumeCapacity(try bincode.read(allocator, list_info.Elem, reader, params));
@@ -51,41 +51,37 @@ pub fn arrayListFieldConfig(comptime ArrayListType: type) bincode.FieldConfig(Ar
     };
 }
 
-pub fn defaultArrayListUnmanagedOnEOFConfig(comptime T: type) bincode.FieldConfig(std.ArrayListUnmanaged(T)) {
+/// Defaults the field of type `List` to an empty state on EOF.
+pub fn defaultOnEofConfig(comptime List: type) bincode.FieldConfig(List) {
+    const al_info = arrayListInfo(List) orelse @compileError("Expected std.ArrayList[Unmanaged]Aligned(T), got " ++ @typeName(List));
     const S = struct {
-        fn deserialize(allocator: std.mem.Allocator, reader: anytype, params: bincode.Params) anyerror!std.ArrayListUnmanaged(T) {
+        fn deserialize(allocator: std.mem.Allocator, reader: anytype, params: bincode.Params) anyerror!List {
             const len = if (bincode.readIntAsLength(usize, reader, params)) |maybe_len|
                 (maybe_len orelse return error.ArrayListTooBig)
-            else |err| {
-                if (err == error.EndOfStream) return .{};
-                return err;
+            else |err| switch (err) {
+                error.EndOfStream,
+                => return switch (al_info.management) {
+                    .managed => List.init(allocator),
+                    .unmanaged => .{},
+                },
+                else => |e| return e,
             };
 
-            const slice = try allocator.alloc(T, len);
+            const slice = try allocator.alignedAlloc(al_info.Elem, al_info.alignment, len);
             errdefer allocator.free(slice);
 
-            return std.ArrayListUnmanaged(T).fromOwnedSlice(slice);
+            return switch (al_info.management) {
+                .managed => List.fromOwnedSlice(allocator, slice),
+                .unmanaged => List.fromOwnedSlice(slice),
+            };
         }
 
         fn free(allocator: std.mem.Allocator, data: anytype) void {
-            data.deinit(allocator);
-        }
-    };
-
-    return .{
-        .deserializer = S.deserialize,
-        .free = S.free,
-    };
-}
-pub fn defaultArrayListOnEOFConfig(comptime T: type) bincode.FieldConfig(std.ArrayList(T)) {
-    const S = struct {
-        fn deserialize(allocator: std.mem.Allocator, reader: anytype, params: bincode.Params) anyerror!std.ArrayList(T) {
-            var unmanaged = try defaultArrayListUnmanagedOnEOFConfig(T).deserializer.?(allocator, reader, params);
-            return unmanaged.toManaged(allocator);
-        }
-
-        fn free(_: std.mem.Allocator, data: anytype) void {
-            data.deinit();
+            var copy = data;
+            switch (al_info.management) {
+                .managed => copy.deinit(),
+                .unmanaged => copy.deinit(allocator),
+            }
         }
     };
 

src/bincode/bincode.zig

@@ -547,20 +547,8 @@ pub fn free(allocator: std.mem.Allocator, value: anytype) void {
                 }
             } else inline for (info.fields) |field| {
                 if (getFieldConfig(T, field)) |field_config| {
-                    if (field_config.free) |free_fcn| {
-                        var field_value = @field(value, field.name);
-                        switch (@typeInfo(field.type)) {
-                            .Pointer => |*field_info| {
-                                // TODO: Why do we do only slice?
-                                if (field_info.size == .Slice) {
-                                    free_fcn(allocator, field_value);
-                                }
-                            },
-                            else => {
-                                free_fcn(allocator, &field_value);
-                            },
-                        }
-
+                    if (field_config.free) |freeFn| {
+                        freeFn(allocator, @field(value, field.name));
                         continue;
                     }
                 }
@@ -631,7 +619,7 @@ pub fn getConfig(comptime T: type) ?FieldConfig(T) {
     return if (comptime hashMapInfo(T) != null)
         hashmap.hashMapFieldConfig(T, .{})
     else if (comptime arrayListInfo(T) != null)
-        arraylist.arrayListFieldConfig(T)
+        arraylist.standardConfig(T)
     else
         null;
 }
@@ -661,8 +649,8 @@ pub fn getSerializedSizeWithSlice(slice: []u8, data: anytype, params: Params) !u
     return ser_slice.len;
 }
 
-pub fn writeToArray(alloc: std.mem.Allocator, data: anytype, params: Params) !std.ArrayList(u8) {
-    var array_buf = try std.ArrayList(u8).initCapacity(alloc, 2048);
+pub fn writeToArray(allocator: std.mem.Allocator, data: anytype, params: Params) !std.ArrayList(u8) {
+    var array_buf = try std.ArrayList(u8).initCapacity(allocator, 2048);
     try bincode.write(array_buf.writer(), data, params);
 
     return array_buf;
@@ -741,11 +729,10 @@ test "bincode: custom enum" {
 }
 
 test "bincode: default on eof" {
-    const defaultArrayListOnEOFConfig = @import("../sig.zig").bincode.arraylist.defaultArrayListOnEOFConfig;
     const Foo = struct {
         value: u8 = 0,
         accounts: std.ArrayList(u64),
-        pub const @"!bincode-config:accounts" = defaultArrayListOnEOFConfig(u64);
+        pub const @"!bincode-config:accounts" = arraylist.defaultOnEofConfig(std.ArrayList(u64));
         pub const @"!bincode-config:value" = int.defaultOnEof(u8, 0);
     };
 

src/bincode/hashmap.zig

@@ -73,10 +73,11 @@ pub fn hashMapFieldConfig(
         }
 
         fn free(allocator: std.mem.Allocator, data: anytype) void {
+            var copy = data;
             if (hm_info.management == .managed) {
-                data.deinit();
+                copy.deinit();
             } else {
-                data.deinit(allocator);
+                copy.deinit(allocator);
             }
         }
     };