feat(primitives): new type for sector number

[cernicc]

Description

Introduce a new type for SectorNumber that is used throughout the workspace. When you hold an instance of the SectorNumber you can know that the SectorNumber is in bounds and no additional checking is needed. The downside is, that there is much juggling between u64 and SectorNumber in tests. Outside of tests, the usage is almost similar as it was before.

The pr is big because of changes in tests.

Important

Let's break down the implementation and see if the safety benefits outweigh the additional complexity.

The SectorNumber needs to implement Decode. It's used by the runtime when decoding scale format to some runtime specific type. Implementation forwards the input to the u64 decoder and tries to initialize the SectorNumber.

If the decoding fails there is a nice error log on the node achieved by adding chain-error to the codec:
Bad input data provided to validate_transaction: Could not decode `RuntimeCall::StorageProvider.0`: Could not decode `Call::pre_commit_sectors::sectors`: Could not decode `SectorPreCommitInfo::sector_number`: Sector number is too large

The frontend gets a more cryptic error. I didn't find any approach that would made this error better. I think this is also a side effect of some unrelated error that happens while processing a block:

{
    "jsonrpc": "2.0",
    "error": {
        "code": 1002,
        "message": "Verification Error: Runtime error: Execution failed: Execution aborted due to trap: wasm trap: wasm `unreachable` instruction executed\nWASM backtrace:\nerror while executing at wasm backtrace:\n    0: 0x4354b2 - <unknown>!rust_begin_unwind\n    1: 0x3f877 - <unknown>!core::panicking::panic_fmt::h7d3847f6b13ee8e3\n    2: 0x1e08a7 - <unknown>!TaggedTransactionQueue_validate_transaction",
        "data": "RuntimeApi(\"Execution failed: Execution aborted due to trap: wasm trap: wasm `unreachable` instruction executed\\nWASM backtrace:\\nerror while executing at wasm backtrace:\\n    0: 0x4354b2 - <unknown>!rust_begin_unwind\\n    1: 0x3f877 - <unknown>!core::panicking::panic_fmt::h7d3847f6b13ee8e3\\n    2: 0x1e08a7 - <unknown>!TaggedTransactionQueue_validate_transaction\")"
    },
    "id": 51
}

The sector number also implements scale_decode::IntoVisitor and scale_decode::Visitor which is used by the subxt to decode types. This implementation replaces the auto derive of DecideAsType.

Checklist

• Make sure that you described what this change does.
• Have you tested this solution?
• Did you document new (or modified) APIs?

[jmg-duarte]

LGTM but some food for thought

[jmg-duarte]

🚀