Redefine Address as `U128` + `Identity` and Hash as `U256` #1616

Centril · 2024-08-20T11:29:14Z

Description of Changes

Redefines Address as u128.

API and ABI breaking changes

Breaks the stored type of Address in system tables and in user tables.

crates/bindings-csharp/BSATN.Runtime/Builtins.cs

RReverser

C# LGTM modulo one final nit. Haven't looked at Rust much.

crates/lib/tests/serde.rs

crates/lib/src/identity.rs

crates/bindings-csharp/BSATN.Runtime/Builtins.cs

RReverser · 2024-09-05T17:42:22Z

I think in the face of Tyler's proposal that changes identity/address representation altogether, this PR might become obsolete.

Centril · 2024-09-09T09:00:36Z

@RReverser My understanding is that the Identity type will still be a 32 byte value and in that case, this still makes it more efficient and more amenable to fast access through indices. The Address changes will become obsolete though. I'm holding off on more work on this PR until the picture becomes clearer.

kazimuth · 2024-10-03T17:00:31Z

I got the Rust part of the PR mostly rebased, still need to do the C# part. If there are open design discussions remaining we should probably have them.

bfops

CLI changes LGTM

kazimuth · 2024-10-15T19:40:49Z

This is now fully ready for merge together with https://github.com/clockworklabs/SpacetimeDBPrivate/pull/1060 @Centril

Centril · 2024-10-15T21:10:37Z

This is now fully ready for merge together with clockworklabs/SpacetimeDBPrivate#1060 @Centril

PR looks good, but did you give any thought to the formatting changes of identity? I personally think they are an improvement, but ymmv.

kazimuth · 2024-10-17T17:40:57Z

PR looks good, but did you give any thought to the formatting changes of identity? I personally think they are an improvement, but ymmv.

Ah, see Ingvar's comment above.

Ah the Rust implementation probably needs to revert to original hex stringification as well. We do use identity/address strings in a few places (e.g. configs), so this would be a breaking change.

This is a breaking change anyway but it's a wipe-the-database breaking change rather than a rewrite-your-configs breaking change at the moment.

Centril · 2024-10-18T12:10:38Z

PR looks good, but did you give any thought to the formatting changes of identity? I personally think they are an improvement, but ymmv.

Ah, see Ingvar's comment above.

Ah the Rust implementation probably needs to revert to original hex stringification as well. We do use identity/address strings in a few places (e.g. configs), so this would be a breaking change.

This is a breaking change anyway but it's a wipe-the-database breaking change rather than a rewrite-your-configs breaking change at the moment.

In that case, I think this is good to merge, but I cannot approve my own PR... :)

Co-authored-by: James Gilles <jameshgilles@gmail.com> Fix C#? Fix codegen tests Wipe old bench data before running bench tests Fix csharp? Revert U128/U256 formatting changes, they were printing numbers backwards Fix failing test Fix some passing tests, add a failing test Bump serde_json version and add arbitrary_precision feature. This allows passing u128 through serde_json::Value, which is done in the CLI crate.

Centril force-pushed the centril/bytes-sink-write branch from 98942b8 to 2a9ae03 Compare August 20, 2024 11:30

Centril force-pushed the centril/address-is-u128 branch from 6c4370c to 89bfb29 Compare August 20, 2024 11:31