Skip to content

Commit 96fe4c7

Browse files
committed
docs: add BitGoWasm API design conventions
document 7 architectural patterns enforced in code reviews: - prefer Uint8Array, avoid unnecessary base conversions - bigint for monetary amounts (conversions are caller's responsibility) - as const arrays for union types, not magic strings - builders return Transaction objects, not bytes - parsing separate from Transaction (standalone parseTransaction) - use wrapper classes over raw WASM bindings - consistent wrapper API (fromBytes, toBytes, getId, get wasm()) distilled from recurring review comments across wasm-utxo, wasm-solana, and wasm-dot. ref: PR #145 discussion.
1 parent f0a75ac commit 96fe4c7

1 file changed

Lines changed: 269 additions & 0 deletions

File tree

CONVENTIONS.md

Lines changed: 269 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,269 @@
1+
# BitGoWasm Code Conventions
2+
3+
This file documents API design and architecture patterns from code reviews. Following these conventions prevents review churn and keeps the codebase consistent across wasm-utxo, wasm-solana, wasm-mps, and future packages.
4+
5+
These are hard rules, not suggestions. If you're unsure about a pattern, check the existing implementations in wasm-utxo.
6+
---
7+
8+
## 1. Prefer Uint8Array, avoid unnecessary base conversions
9+
10+
**What:** Generally, binary formats like transactions should use `Uint8Array` . Avoid base conversions in the wasm interface.
11+
12+
**Why:** Type safety at the boundary. If a method accepts or returns transaction bytes, it's always `Uint8Array`. String encodings (hex, base64) belong in serialization/API layers, not in the core transaction model. This prevents API bloat where we have to add encoding and decoding variants for various base conversion formats, as well as inefficiencies due to round-tripping binary data through two base conversions.
13+
14+
Exceptions are allowed for instances where a hex encoded representations are conventional (Ethereum addresses for instance)
15+
16+
**Good:**
17+
```typescript
18+
class Transaction {
19+
static fromBytes(bytes: Uint8Array): Transaction { ... }
20+
toBytes(): Uint8Array { ... }
21+
signablePayload(): Uint8Array { ... }
22+
}
23+
24+
// Encoding happens at the boundary
25+
const txBytes = Buffer.from(txHex, 'hex');
26+
const tx = Transaction.fromBytes(txBytes);
27+
```
28+
29+
**Bad:**
30+
```typescript
31+
// ❌ Don't accept/return hex strings on Transaction
32+
class Transaction {
33+
static fromHex(hex: string): Transaction { ... }
34+
toHex(): string { ... }
35+
}
36+
37+
// ❌ Don't mix encodings
38+
static fromBytes(bytes: Uint8Array | string): Transaction { ... }
39+
```
40+
41+
**See:** `packages/wasm-solana/js/transaction.ts`, `packages/wasm-utxo/js/transaction.ts`
42+
43+
---
44+
45+
## 2. bigint for amounts, never string
46+
47+
**What:** All monetary amounts, lamports, satoshis, token quantities, fees — use `bigint`. Never `number` or `string`.
48+
49+
**Why:**
50+
- `number` loses precision above 2^53 (unsafe for large amounts)
51+
- `string` delays type errors to runtime (no compile-time safety)
52+
- `bigint` is exact, type-safe, and enforces correctness at compile time
53+
54+
Conversions between external representations (API strings, JSON numbers) and `bigint` are the caller's responsibility, outside the `wasm-*` package boundary. The wasm package API accepts and returns `bigint` only — no `string` or `number` overloads for amounts.
55+
56+
**Good:**
57+
```typescript
58+
export interface ExplainedOutput {
59+
address: string;
60+
amount: bigint; //
61+
}
62+
63+
const fee = 5000n;
64+
const total = amount + fee; // Type-safe bigint arithmetic
65+
```
66+
67+
**Bad:**
68+
```typescript
69+
export interface ExplainedOutput {
70+
address: string;
71+
amount: string; // ❌ Runtime errors, no type safety
72+
}
73+
74+
const fee = "5000"; // ❌ Can't do arithmetic
75+
const total = parseInt(amount) + parseInt(fee); // ❌ Loses precision
76+
```
77+
78+
**See:** `packages/wasm-solana/js/explain.ts` (lines 40-43), `CLAUDE.md`
79+
80+
---
81+
82+
## 3. Const arrays for union types, not magic strings
83+
84+
**What:** Use `as const` arrays to define finite sets of known values. Never use bare string literals for types, opcodes, instruction names, etc.
85+
86+
**Why:**
87+
- Compile-time checking (typos caught at build time)
88+
- IDE autocomplete
89+
- Exhaustiveness checking in switch statements
90+
- Less repetitive than `enum` (no `Key = "Key"` duplication)
91+
92+
**Good:**
93+
```typescript
94+
export const TransactionType = ["Send", "StakingActivate", "StakingDeactivate"] as const;
95+
export type TransactionType = typeof TransactionType[number];
96+
97+
function handleTx(type: TransactionType) {
98+
switch (type) {
99+
case "Send":
100+
// ...
101+
case "StakingActivate":
102+
// ...
103+
// TypeScript warns if you miss a case
104+
}
105+
}
106+
```
107+
108+
**Bad:**
109+
```typescript
110+
// ❌ No type safety, typos not caught
111+
function handleTx(type: string) {
112+
if (type === "send") { // Oops, wrong case
113+
// ...
114+
}
115+
}
116+
117+
// ❌ Magic strings scattered everywhere
118+
const txType = "Send";
119+
```
120+
121+
**See:** `packages/wasm-solana/js/explain.ts` (lines 19-28)
122+
123+
---
124+
125+
## 4. Return Transaction objects, not bytes (builders)
126+
127+
**What:** Builder functions and transaction constructors return `Transaction` objects, not raw `Uint8Array`. The caller serializes when they need bytes.
128+
129+
**Why:** Transaction objects can be inspected and further modified (`.addSignature()`, `.signWithKeypair()`). Returning bytes forces the caller to re-parse if they need to inspect or modify.
130+
131+
**Good:**
132+
```typescript
133+
export function buildFromIntent(params: BuildParams): Transaction {
134+
const wasm = BuilderNamespace.build_from_intent(...);
135+
return Transaction.fromWasm(wasm);
136+
}
137+
138+
// Caller has full control
139+
const tx = buildFromIntent(intent);
140+
console.log(tx.feePayer); // Inspect
141+
tx.addSignature(pubkey, sig); // Modify
142+
const bytes = tx.toBytes(); // Serialize when ready
143+
```
144+
145+
**Bad:**
146+
```typescript
147+
// ❌ Forces caller to re-parse for inspection
148+
export function buildFromIntent(params: BuildParams): Uint8Array {
149+
const wasm = BuilderNamespace.build_from_intent(...);
150+
return wasm.to_bytes();
151+
}
152+
153+
const bytes = buildFromIntent(intent);
154+
const tx = Transaction.fromBytes(bytes); // Unnecessary round-trip
155+
```
156+
157+
**See:** `packages/wasm-solana/js/intentBuilder.ts`, `packages/wasm-solana/js/builder.ts`
158+
159+
---
160+
161+
## 5. Parsing separate from Transaction
162+
163+
**What:** Transaction deserialization (for signing) and transaction parsing (decoding instructions) are separate operations with separate entry points. `Transaction.fromBytes()` deserializes for signing. `parseTransaction()` is a standalone function that decodes a `Transaction` into structured data.
164+
165+
**Why:**
166+
- Separation of concerns: deserialization is a protocol-level concept, parsing is a BitGo-level concept
167+
- `parseTransaction` accepts a `Transaction` object (not raw bytes) to avoid double-deserialization — the caller typically already has a `Transaction` from `fromBytes()` for the signing flow
168+
169+
**Good:**
170+
```typescript
171+
// Typical flow: decode once, use for both parsing and signing
172+
const tx = Transaction.fromBytes(txBytes);
173+
const parsed = parseTransaction(tx);
174+
if (!validateParsed(parsed, buildParams)) { throw new Error(); }
175+
tx.addSignature(pubkey, signature);
176+
const signedBytes = tx.toBytes();
177+
178+
// Parsed data is for inspection only
179+
for (const instr of parsed.instructionsData) {
180+
if (instr.type === 'Transfer') {
181+
console.log(`${instr.amount} to ${instr.toAddress}`);
182+
}
183+
}
184+
```
185+
186+
**Bad:**
187+
```typescript
188+
// ❌ Don't accept raw bytes — forces redundant deserialization
189+
const parsed = parseTransaction(txBytes);
190+
191+
// ❌ Transaction does not have a .parse() method
192+
const tx = Transaction.fromBytes(txBytes);
193+
const parsed = tx.parse(); // Doesn't exist
194+
195+
// ❌ Don't use parseTransaction result for signing
196+
const parsed = parseTransaction(tx);
197+
parsed.addSignature(pubkey, sig); // Wrong object type
198+
```
199+
200+
**See:** `packages/wasm-solana/js/parser.ts` (parseTransaction function), `packages/wasm-solana/js/transaction.ts` (Transaction.fromBytes), `packages/wasm-dot/js/parser.ts`
201+
202+
---
203+
204+
## 6. Use wrapper classes
205+
206+
**What:** Wrap WASM-generated types in TypeScript classes that provide better type signatures, `camelCase` naming, and encapsulation. Don't expose raw WASM bindings to consumers.
207+
208+
**Why:** `wasm-bindgen` emits loose types (`any`, `string | null`) and `snake_case` naming. Wrapper classes provide precise TypeScript types, idiomatic JS naming, and hide WASM implementation details. Two patterns exist: namespace wrappers for stateless utilities, class wrappers for stateful objects.
209+
210+
**See:** [`packages/wasm-utxo/js/README.md`](https://github.com/BitGo/BitGoWASM/blob/master/packages/wasm-utxo/js/README.md#purpose) for the full rationale and examples of both patterns.
211+
212+
---
213+
214+
## 7. Follow wasm-utxo conventions (get wasm(), fromBytes, toBytes, getId)
215+
216+
**What:** All wrapper classes follow the same API pattern:
217+
- `static fromBytes(bytes: Uint8Array)` — deserialize
218+
- `toBytes(): Uint8Array` — serialize
219+
- `getId(): string` — transaction ID / hash
220+
- `get wasm(): WasmType` (internal) — access underlying WASM instance
221+
222+
**Why:**
223+
- Consistency across packages (wasm-utxo, wasm-solana, wasm-mps all work the same way)
224+
- Predictable API for consumers
225+
- `get wasm()` allows package-internal code to access WASM without exposing it publicly
226+
227+
**Good:**
228+
```typescript
229+
export class Transaction {
230+
private constructor(private _wasm: WasmTransaction) {}
231+
232+
static fromBytes(bytes: Uint8Array): Transaction {
233+
return new Transaction(WasmTransaction.from_bytes(bytes));
234+
}
235+
236+
toBytes(): Uint8Array {
237+
return this._wasm.to_bytes();
238+
}
239+
240+
getId(): string {
241+
return this._wasm.id;
242+
}
243+
244+
/** @internal */
245+
get wasm(): WasmTransaction {
246+
return this._wasm;
247+
}
248+
}
249+
```
250+
251+
**Bad:**
252+
```typescript
253+
// ❌ Inconsistent naming
254+
export class Transaction {
255+
static parse(bytes: Uint8Array): Transaction { ... } // Should be fromBytes
256+
serialize(): Uint8Array { ... } // Should be toBytes
257+
getTransactionId(): string { ... } // Should be getId
258+
}
259+
```
260+
261+
**See:** `packages/wasm-utxo/js/transaction.ts`, `packages/wasm-solana/js/transaction.ts`
262+
263+
---
264+
265+
## Summary
266+
267+
These 7 conventions define how BitGoWasm packages structure their APIs. They're architectural patterns enforced in code reviews — not general software practices or build requirements.
268+
269+
When in doubt, look at wasm-solana and wasm-utxo — they're the reference implementations. Following these patterns from the start prevents review churn and keeps all packages consistent.

0 commit comments

Comments
 (0)