Issue 80 #81

shamblett · 2026-01-14T15:06:23Z

No description provided.

AlexDochioiu · 2026-01-14T19:13:17Z

lib/src/value/bytes.dart

 }

-class _CborBytesImpl with CborValueMixin implements CborBytes {
+class CborBytesImpl with CborValueMixin implements CborBytes {


_CborBytesImpl can stay private as long as we add to this file part "typed_data.dart" and part of "bytes.dart for typed_data

I couldn't get this to work,, maybe try again when the code has settles some more.

AlexDochioiu · 2026-01-14T19:14:29Z

lib/src/value/typed_data.dart

+import 'package:ieee754/ieee754.dart';
+
+/// Base class for all Typed Arrays
+abstract class CborTypedArray extends CborBytesImpl {


I'd consider:

Potentially making this private.

Making this sealed

Made sealed, again couldn't get visibility to work as private.

AlexDochioiu · 2026-01-14T19:21:31Z

Review from Claude Code

 ---
  Bug Review: PR #81 - RFC 8746 Typed Array Support

  ✅ Correct Implementation

  The tag numbers (64-87) are correctly defined per RFC 8746. Tag 76 is correctly omitted (it's reserved in the RFC since sint8 has no endianness).

  ---
  🐛 Bugs Found

  1. No Byte Length Validation (Silent Data Loss)

  Files: lib/src/value/typed_data.dart (all typed array classes)

  All typed array implementations silently truncate misaligned data:

  // CborUint16BigEndianArray
  final list = Uint16List(bytes.length ~/ 2);  // 3 bytes → 1 element, 1 byte silently lost

  If someone decodes malformed CBOR with 5 bytes for a uint32 array, only 1 element is created and the 5th byte is silently ignored. This should at minimum log a warning, or ideally throw an error for strict decoding.

  ---
  2. Float16 Byte Order Assumption is Unverified

  File: lib/src/value/typed_data.dart:259-300

  The code assumes FloatParts.fromFloat16Bytes() from the ieee754 package expects big-endian bytes:

  // Big Endian - passes bytes as-is
  final chunk = Uint8List.fromList([
    data.getUint8(i),
    data.getUint8(i + 1),
  ]);

  // Little Endian - swaps bytes
  final chunk = Uint8List.fromList([
    data.getUint8(i + 1),  // swapped
    data.getUint8(i),
  ]);

  The code comment even admits uncertainty: "If it's BE, bytes are BE... I'll assume FloatParts expects Big Endian". If this assumption is wrong, both Float16 implementations produce incorrect values. This needs verification against the ieee754 package documentation.

  ---
  3. Float128 Arrays Return Raw Bytes Instead of Meaningful Result

  File: lib/src/value/typed_data.dart:348-366

  class CborFloat128BigEndianArray extends CborTypedArray {
    const CborFloat128BigEndianArray(super.bytes, {super.tags});
    // No native support, return bytes or implement if possible.
  }

  These classes don't override toObjectInternal(), so calling toObject() returns raw bytes via the parent class. This is confusing behavior - a caller expecting a list of numbers gets List<int> bytes instead. Should either:
  - Throw UnimplementedError with a clear message
  - Return null
  - Return a List<BigInt> or other representation

  ---
  4. CborInt8Array Comment Contains Dead Code/Confusion

  File: lib/src/value/typed_data.dart:153-163

  @override
  Object? toObjectInternal(Set<Object> cyclicCheck, ToObjectOptions o) {
    return Int8List.fromList(
      bytes,
    ); // Reinterprets bytes? No, fromList copies values.
    // Bytes are 0-255. Int8List expects -128 to 127.
    // We should use Int8List.view if possible or convert.
    // Uint8List can be viewed as Int8List.
  }

  The trailing comments suggest confusion about whether this works correctly. The implementation is actually correct (Int8List.fromList([255]) properly becomes [-1]), but the confusing comments should be removed.

  ---
  ⚠️ Design Issues / Missing Features

  5. No Encoding Support

  The PR only implements decoding. There's no convenient way to create typed arrays for encoding. Users must manually:
  final bytes = Uint8List(4);
  ByteData.view(bytes.buffer).setUint16(0, value, Endian.big);
  final array = CborUint16BigEndianArray(bytes, tags: [65]);

  This is error-prone. Consider adding factory constructors:
  factory CborUint16BigEndianArray.fromList(List<int> values) { ... }

  ---
  6. Incomplete Test Coverage

  File: test/rfc8746_test.dart

  Missing tests for:
  - CborUint64BigEndianArray / CborUint64LittleEndianArray
  - CborInt32BigEndianArray / CborInt32LittleEndianArray
  - CborInt64BigEndianArray / CborInt64LittleEndianArray
  - CborUint8ClampedArray
  - CborFloat128BigEndianArray / CborFloat128LittleEndianArray
  - Edge cases: empty arrays, arrays with odd byte counts, maximum values

  ---
  7. Missing Multi-dimensional Array Support (Tags 40, 41, 1040)

  RFC 8746 also defines tags for multi-dimensional arrays which aren't implemented. This might be intentional scope limitation but should be documented.

  ---
  Summary Table
  Issue: Silent truncation of misaligned bytes
  Severity: Medium
  File: typed_data.dart
  Line: All classes
  ────────────────────────────────────────
  Issue: Unverified Float16 byte order
  Severity: Medium
  File: typed_data.dart
  Line: 259-300
  ────────────────────────────────────────
  Issue: Float128 returns bytes not error
  Severity: Low
  File: typed_data.dart
  Line: 348-366
  ────────────────────────────────────────
  Issue: Confusing dead comments in Int8Array
  Severity: Low
  File: typed_data.dart
  Line: 153-163
  ────────────────────────────────────────
  Issue: No encoding helpers
  Severity: Low
  File: typed_data.dart
  Line: -
  ────────────────────────────────────────
  Issue: Incomplete test coverage
  Severity: Low
  File: rfc8746_test.dart
  Line: -

AlexDochioiu · 2026-01-14T19:46:07Z

lib/src/value/typed_data.dart

+/// sint64 big endian
+class CborInt64BigEndianArray extends CborTypedArray {


ByteData's getUint64() and getInt64() (and the setters) do not work on web. They throw a runtime exception. It works well on wasm.

I actually had to deal with this issue just a few weeks ago for a package that I maintain. See diff here:

vespr-wallet/cardano_dart_types@014b763#diff-868a434c4475349d43ff872a00e37054236d21bfa69a6bfa1da37529f78c198aR191

--
Since JS has no distinction between int and float, you can use setFloat64 and getFloat64 instead. However, anything above JS's Number.MAX_SAFE_INTEGER may be corrupted (changed). In other words, it's not a bulletproof solution.

So, the way I see, there's two options:

Simple fix: Check for web js and use Float64 while accepting it breaks for (some) large numbers

Complex fix: use BigInt which would probably cause a bigger change

Note: Web WASM supports int64 and uint64 types. However, I am not sure if ByteData's get/setUint64 and get/setInt64 throws on web js only, or also on web wasm. It would need checking.

My suggestion would be to do (for now) the simple fix and maybe leave an issue open and a comment to the issue as a known limitation.

If ByteData's int64's operations do not work on WASM, the same can be achieved with two reads/writes of int32 operations and some binary shift, which should be perfectly safe to use for both VM and WEB WASM, while WEB JS can use the float64 operation(s).

L.e. Those are the files (config + git workflow) to run tests for VM + WEB JS + WEB WASM in CI if you're interested.

https://github.com/vespr-wallet/cardano_dart_types/blob/main/dart_test.yaml

https://github.com/vespr-wallet/cardano_dart_types/blob/main/.github/workflows/mobile-tests.yml

Great thanks for the comprehensive review, especially the web/wasm stuff. I've come across this before where things work OK in web(JS) but not wasm and vice versa in some of my other packages. I think the Google guys still have some work to do here.

I'll look at this seperately.

The ramifications of updating the package to web/wasm targets is a bit more than just for this RFC, other tests concerning large numbers, infinity etc. also fail when tested in a web environment which is to be expected of course.

I've raised #82 to address web/wasm usage as a seperate task.

AlexDochioiu

Pretty solid job. Main issue I see is about web (especially js) support

Key changes: 1. Validation: Added checks in toObjectInternal to throw CborMalformedException if the byte length is not a multiple of the element size. 2. Endianness: Fixed Float16 Little Endian handling by reversing bytes before decoding. 3. Float128: Updated toObjectInternal to throw UnimplementedError instead of returning raw bytes. 4. Int8: Removed confusing comments. 5. Factories: Added fromList factory constructors to Typed Array classes to facilitate encoding (e.g., CborUint16BigEndianArray.fromList([1, 2])). 6. Tests: Expanded test/rfc8746_test.dart to cover missing types (Uint64, Int32, Float128), edge cases (misaligned length), and the new factories.

shamblett · 2026-01-16T15:03:10Z

Gemini updates so far on the review. My comments added in line. I asked it not to look at the web/wasm stuff I'll look at this.

AlexDochioiu · 2026-01-16T19:29:08Z

I looked over the new commits. Looks good to me!

shamblett · 2026-01-16T20:14:41Z

Great, thanks

… of RFC8746. Summary of changes: 1. Added Tag 40: Added CborTag.multiDimensionalArray to lib/src/value/value.dart. 2. Created `CborMultiDimensionalArray`: Implemented the CborMultiDimensionalArray class in lib/src/value/multi_dimensional_array.dart, including encode, toObject, and toJson methods. 3. Added Factory Constructors: Added fromTypedArray and fromValues factory constructors to CborMultiDimensionalArray for easier instantiation. 4. Updated Decoder: Updated lib/src/decoder/stage3.dart to correctly decode arrays with Tag 40 into CborMultiDimensionalArray objects. 5. Fixed `isHintSubtype`: Updated lib/src/utils/utils.dart to include multiDimensionalArray (and missing rationalNumber, decimalFraction) in isHintSubtype, enabling proper tag detection. 6. Added Tests: Added comprehensive unit tests for encoding and decoding multi-dimensional arrays in test/rfc8746_test.dart. Verified all tests passed with dart test.

shamblett added 3 commits January 14, 2026 15:05

Issue 80

326340c

Issue 80

9cb30d5

Issue 80

a520fb9

shamblett mentioned this pull request Jan 14, 2026

Incorporate RFC 8746 #80

Closed

AlexDochioiu reviewed Jan 14, 2026

View reviewed changes

shamblett added 2 commits January 14, 2026 20:24

Issue 80

5d27f32

AlexDochioiu approved these changes Jan 16, 2026

View reviewed changes

shamblett added 3 commits January 17, 2026 11:02

Formatting

6a5b8a0

README add RFC8764 incorporated

4649273

shamblett mentioned this pull request Jan 18, 2026

Update the package to be web and wasm capable. #82

Closed

shamblett merged commit aac1102 into master Jan 18, 2026
2 checks passed

		/// sint64 big endian
		class CborInt64BigEndianArray extends CborTypedArray {

Issue 80 #81

Issue 80 #81

Uh oh!

Conversation

shamblett commented Jan 14, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlexDochioiu commented Jan 14, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlexDochioiu left a comment

Choose a reason for hiding this comment

Uh oh!

shamblett commented Jan 16, 2026

Uh oh!

AlexDochioiu commented Jan 16, 2026

Uh oh!

shamblett commented Jan 16, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants