fix(grpc): addAddressPadding on worldAddress #500
Conversation
WalkthroughAdds Starknet address padding by introducing the Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant SDK as SDK (createSDK)
participant Client as ToriiGrpcClient
participant Mapping as mapWorldAddresses
participant GRPC as gRPC service
SDK->>Client: receive config.client.worldAddress
Note right of Client: new: addAddressPadding(worldAddress)
SDK->>Client: pass padded worldAddress
Client->>Mapping: request world address conversion
Mapping->>Mapping: addAddressPadding(address)
Mapping->>Mapping: hexToBuffer(paddedAddress)
Mapping->>GRPC: send buffered address
GRPC-->>Client: response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (4)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/grpc/src/torii-client.test.ts (1)
7-7: Consider testing the address padding behavior.The tests now include
worldAddressin the config, but there's no test verifying thataddAddressPaddingis actually applied. Consider adding a test that checks the padded address format.Example test to add:
it("should pad world address on construction", () => { const client = new ToriiGrpcClient({ toriiUrl: "http://localhost:8080", worldAddress: "0x1", }); // @ts-ignore - accessing private property for testing expect(client.worldAddress).toBe("0x0000000000000000000000000000000000000000000000000000000000000001"); });packages/sdk/src/createSDK.ts (1)
237-237: <!-- [scratchpad_update]
[observations]
- Web search returned general starknet.js docs but no specific info on addAddressPadding idempotency
- The function appears in examples but implementation details not in search results
- Need to search more specifically for the function implementation or behavior
[actions]- Search for addAddressPadding source code or specific documentation
[scratchpad_end] --><!-- [scratchpad_update]
[observations]- Found starknet.js API docs confirming addAddressPadding signature
- Doc states: "Format a hex number to '0x' and 64 characters, adding leading zeros if necessary"
- Example shows function adds leading zeros to short addresses
- Key: function pads with leading zeros to create 64-character format
[analysis]- addAddressPadding converts any hex address to a padded format (0x + 64 chars)
- If already padded (64 chars), adding leading zeros to a 64-char string results in same 64-char string (idempotent)
- Example: '0x1234...' becomes '0x0000...1234...' (padded to 64), calling again should return same result
- This means it IS idempotent - calling multiple times returns same result
[observations_continued]- Script output confirms duplicate padding in the codebase
- Review comment states it's idempotent and safe but redundant
- Need to verify if this is truly a recommended optional refactor
[actions]- Look at the actual behavior: check if function checks already-padded addresses
- Determine if removing duplicate padding would be a clean refactor
[scratchpad_end] -->Based on the verified script output and documentation, I can now provide the rewritten review comment:
Remove redundant address padding in createSDK.ts.
The
addAddressPaddingfunction formats a hex number to '0x' and 64 characters, adding leading zeros if necessary, which makes it idempotent. The worldAddress is padded twice: increateSDK.tsline 237 before passing toToriiGrpcClient, and again inToriiGrpcClient's constructor attorii-client.tsline 253.Remove the padding in
createSDK.tsline 237 and let theToriiGrpcClientconstructor handle it.new ToriiGrpcClient({ toriiUrl: config.client.toriiUrl ?? "http://localhost:8080", - worldAddress: addAddressPadding(config.client.worldAddress), + worldAddress: config.client.worldAddress,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
packages/grpc/package.json(1 hunks)packages/grpc/src/mappings/query.ts(2 hunks)packages/grpc/src/torii-client.test.ts(2 hunks)packages/grpc/src/torii-client.ts(3 hunks)packages/sdk/src/createSDK.ts(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build-examples
- GitHub Check: build-packages
- GitHub Check: build
🔇 Additional comments (8)
packages/grpc/package.json (1)
40-40: LGTM! Starknet dependency added correctly.The addition of the
starknetdependency is appropriate for usingaddAddressPadding. Thecatalog:version specifier indicates proper workspace-level version management.packages/grpc/src/mappings/query.ts (2)
79-79: LGTM! Import added correctly.
319-326: LGTM! Address padding applied correctly in query mapping.The padding is applied before hex-to-buffer conversion, ensuring world addresses in queries are consistently formatted. This function is used throughout the query mapping logic.
packages/sdk/src/createSDK.ts (1)
3-3: LGTM! Import added correctly.packages/grpc/src/torii-client.ts (3)
130-130: LGTM! Import added correctly.
253-253: LGTM! Address padding applied in constructor.The worldAddress from config is padded when the client is constructed, ensuring consistent address formatting throughout the client's lifetime.
1042-1045: LGTM! Address padding applied for runtime world addresses.The padding is correctly applied to world addresses passed dynamically to
getWorlds, which is separate from the config worldAddress handled in the constructor.packages/grpc/src/torii-client.test.ts (1)
54-54: LGTM! Test config updated with worldAddress.The test correctly includes the new
worldAddressfield, maintaining backwards compatibility testing.
MartianGreed
force-pushed
the
fix/worldaddress
branch
from
b9f7335 to
f02b07e
Compare
Closes #
Introduced changes
Checklist
Summary by CodeRabbit
New Features
worldAddressconfiguration parameter for client setup.Dependencies