feat: add authz and feegrant support #103
Conversation
Add decoders for Cosmos authz and feegrant modules with comprehensive test coverage. - Implement authz decoder for grant, revoke, and exec operations - Implement feegrant decoder for allowance grant and revoke operations - Add schema definitions and message type constants - Add comprehensive test fixtures and test cases
WalkthroughThis PR adds support for decoding Cosmos SDK authz and feegrant messages, including MsgExec, MsgGrant, MsgRevoke, MsgGrantAllowance, and MsgRevokeAllowance. The decoder selection logic has been refactored to use a helper method. New schemas, interfaces, decoders, and comprehensive tests are included. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Decoder
participant DecoderHelper
participant AuthzExecDecoder
participant InnerDecoders
Caller->>Decoder: _decodeMessage(msg, vm)
Decoder->>DecoderHelper: _getDecodersForVm(vm)
DecoderHelper-->>Decoder: MessageDecoder[]
alt Message is MsgExec
Decoder->>AuthzExecDecoder: decode(msg, this._getDecodersForVm.bind(this))
AuthzExecDecoder->>AuthzExecDecoder: validate with zMsgExec
loop for each inner message in MsgExec.msgs
AuthzExecDecoder->>AuthzExecDecoder: resolveDecoder(innerMsg)
AuthzExecDecoder->>InnerDecoders: decode(innerMsg, getDecodersForVm)
InnerDecoders-->>AuthzExecDecoder: DecodedMessage
end
AuthzExecDecoder-->>Decoder: DecodedAuthzExecMessage
else Message is other Authz/Feegrant
Decoder->>AuthzExecDecoder: decode(msg, getDecodersForVm)
AuthzExecDecoder-->>Decoder: DecodedMessage
end
Decoder-->>Caller: DecodedMessage
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Special attention areas:
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. Comment |
tansawit
changed the title
- Clarifies purpose of optional callback in authz exec decoder - Explains recursive decoding of nested messages
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (16)
README.md(1 hunks)src/decoder-registry.ts(2 hunks)src/decoder.ts(2 hunks)src/decoders/cosmos/authz.ts(1 hunks)src/decoders/cosmos/feegrant.ts(1 hunks)src/decoders/cosmos/index.ts(1 hunks)src/interfaces/cosmos.ts(2 hunks)src/interfaces/decoder.ts(1 hunks)src/message-types.ts(1 hunks)src/schema/cosmos/authz.ts(1 hunks)src/schema/cosmos/feegrant.ts(1 hunks)src/schema/cosmos/index.ts(1 hunks)src/tests/cosmos/move/authz-message-types.fixture.ts(1 hunks)src/tests/cosmos/move/authz.fixture.ts(1 hunks)src/tests/cosmos/move/authz.test.ts(1 hunks)src/tests/cosmos/move/feegrant.test.ts(1 hunks)
🔇 Additional comments (9)
src/schema/cosmos/index.ts (2)
1-1: Authz schema export looks good.Re-exporting the authz schemas here keeps downstream consumers in sync with the new decoders.
5-5: Feegrant schema export wired correctly.Including the feegrant barrel ensures the new allowance schemas are available wherever the cosmos schemas are imported.
src/decoders/cosmos/index.ts (2)
1-1: Authz decoder export confirmed.Adding the authz decoders to the barrel keeps the registry wiring simple and consistent.
4-4: Feegrant decoder export confirmed.The feegrant decoder export ensures the new modules are reachable via the existing cosmos decoder surface.
src/decoder-registry.ts (1)
5-33: Decoder registry additions look correct.Prepending the authz and feegrant decoders across the VM-specific registries ensures those messages are discoverable no matter which Cosmos VM path we're decoding.
src/decoder.ts (2)
227-240: Helper extraction enables recursive decoding.Refactoring the VM switch into
_getDecodersForVmand passing the bound helper todecoder.decodegives nested decoders everything they need without duplicating logic. Nice cleanup.
293-304: _getDecodersForVm helper looks solid.Centralizing the VM→decoder mapping here reduces duplication and makes it easy to extend for new VM types later.
src/decoders/cosmos/authz.ts (1)
46-67: Thoughtful recursion wiring.Passing
getDecodersForVmthrough letsMsgExecpeel nested payloads cleanly while still falling back to graceful “not supported” stubs when nothing matches, which keeps exec decoding resilient across VMs.src/tests/cosmos/move/authz.test.ts (1)
499-646: Authz exec sender coverage is spot-onExercising the granter-as-sender case inside MsgExec nails the nuance that most regressions miss. Happy with this.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/decoders/cosmos/authz.ts (1)
54-67: Handle inner decoder failures gracefully.If a nested decoder throws (e.g., schema drift on a known message type), the exception bubbles up and aborts the entire MsgExec decode. Wrapping the call and falling back to
createNotSupportedMessagewill preserve the remaining inner messages while still signalling the unsupported one.- if (decoder) { - const decoded = await decoder.decode( - innerMessage, - log, - apiClient, - txResponse, - vm, - getDecodersForVm - ); - decodedInnerMessages.push(decoded); - } else { + if (decoder) { + try { + const decoded = await decoder.decode( + innerMessage, + log, + apiClient, + txResponse, + vm, + getDecodersForVm + ); + decodedInnerMessages.push(decoded); + } catch { + decodedInnerMessages.push( + createNotSupportedMessage(innerMessage["@type"]) + ); + } + } else {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (16)
README.md(1 hunks)src/decoder-registry.ts(2 hunks)src/decoder.ts(2 hunks)src/decoders/cosmos/authz.ts(1 hunks)src/decoders/cosmos/feegrant.ts(1 hunks)src/decoders/cosmos/index.ts(1 hunks)src/interfaces/cosmos.ts(2 hunks)src/interfaces/decoder.ts(1 hunks)src/message-types.ts(1 hunks)src/schema/cosmos/authz.ts(1 hunks)src/schema/cosmos/feegrant.ts(1 hunks)src/schema/cosmos/index.ts(1 hunks)src/tests/cosmos/move/authz-message-types.fixture.ts(1 hunks)src/tests/cosmos/move/authz.fixture.ts(1 hunks)src/tests/cosmos/move/authz.test.ts(1 hunks)src/tests/cosmos/move/feegrant.test.ts(1 hunks)
🧰 Additional context used
🪛 Gitleaks (8.28.0)
src/tests/cosmos/move/authz.fixture.ts
[high] 369-369: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 893-893: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 1386-1386: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (23)
src/tests/cosmos/move/authz.fixture.ts (1)
1-1474: LGTM! Comprehensive test fixtures for authz scenarios.The fixture file provides thorough test coverage with real blockchain transaction data. The Gitleaks warnings about "API keys" at lines 369, 893, and 1386 are false positives—these are public keys from blockchain transactions, which are safe to include in test fixtures.
src/decoders/cosmos/index.ts (1)
1-1: LGTM! Exports follow the existing pattern.The new exports for authz and feegrant modules are properly integrated into the cosmos decoders index.
Also applies to: 4-4
README.md (1)
355-359: LGTM! Clear documentation of authz support.The documentation accurately describes MsgExec's recursive decoding capability and follows the existing documentation structure.
src/interfaces/decoder.ts (1)
17-18: LGTM! Clean extension for recursive decoding.The optional
getDecodersForVmparameter enables nested message decoding (e.g., MsgExec) while maintaining backward compatibility with existing decoders.src/schema/cosmos/index.ts (1)
1-1: LGTM! Schema exports properly integrated.The authz and feegrant schema exports follow the existing pattern and are correctly positioned in the module hierarchy.
Also applies to: 5-5
src/schema/cosmos/feegrant.ts (1)
1-25: LGTM! Well-structured feegrant schemas.The Zod schemas correctly model Cosmos SDK feegrant messages. The use of
.passthrough()onzAllowanceis appropriate for handling different allowance types (e.g., BasicAllowance, PeriodicAllowance) with varying fields.src/decoder.ts (2)
227-227: LGTM! Clean refactoring for recursive decoding.The refactoring to use
_getDecodersForVmand pass a bound version to decoders enables recursive message decoding (for MsgExec) while keeping the code clean and maintainable.Also applies to: 233-240
293-304: LGTM! Well-structured helper method.The
_getDecodersForVmhelper provides clean VM-to-decoder mapping with appropriate error handling for unknown VM types.src/message-types.ts (1)
26-35: LGTM! Message type constants properly defined.The new authz and feegrant message type constants follow the existing pattern and correctly map to Cosmos SDK message type paths. The trailing comma on line 26 is a minor formatting improvement.
src/decoders/cosmos/feegrant.ts (1)
22-35: Solid validation path here.The
safeParseguard plus the direct handoff of the parsed fields keeps malformed feegrant payloads from ever leaking into the decoded output, which is exactly what we want for these modules.src/tests/cosmos/move/feegrant.test.ts (1)
132-214: Great coverage on allowance variants.Exercising both empty and populated spend limits ensures the decoder won’t regress when allowance shapes vary, and the expectations line up with the schema additions.
src/tests/cosmos/move/authz.test.ts (1)
174-356: Appreciate the multi-message authz scenario.Validating multiple inner MsgSends—including the grantee/granter wiring—gives strong regression protection for nested decoding logic.
src/decoder-registry.ts (1)
5-33: Decoder ordering looks correct.Placing the authz/feegrant handlers ahead of the generic banking decoders guarantees MsgExec and allowance messages short-circuit to the right implementation before the legacy fallbacks run.
src/schema/cosmos/authz.ts (3)
12-21: LGTM: Authorization and grant schemas are appropriately flexible.The
zAuthorizationschema correctly uses.passthrough()to allow additional properties beyond@type, which is necessary since different authorization types have different fields. ThezGrantschema properly makesexpirationoptional.
23-35: LGTM: Grant and revoke schemas look correct.Both
zMsgGrantandzMsgRevokeschemas properly validate their respective message structures with appropriate required fields.
1-10: Review comment is factually incorrect; no issues found.The actual
zMessageschema insrc/schema/cosmos/bank.ts(lines 6-10) is:z.object({ "@type": z.string() }).passthrough()This is a generic, permissive schema that:
- Accepts ANY Cosmos message type (not just bank messages)
- Supports recursive messages via
.passthrough()- Already handles extensibility for future message types
The review comment's concerns about being "too narrow," having "circular dependency risk," and "breaking extensibility" do not apply to the actual implementation. The schema correctly validates
MsgExec.msgsas an array of any supported message type without architectural issues.While the location of
zMessageinbank.ts(rather than a dedicated shared schema file) is a minor organizational concern, the implementation is functionally correct as-is.Likely an incorrect or invalid review comment.
src/interfaces/cosmos.ts (3)
9-11: LGTM: Union type properly extended.The new authz and feegrant message interfaces are correctly added to the
DecodedMessageunion type.Also applies to: 19-20
48-54: LGTM: Recursive message structure correctly implemented.The
DecodedAuthzExecMessageinterface properly usesDecodedMessage[]for the nested messages, which correctly handles the recursive nature of authz exec operations where one message can contain other decoded messages of any type.
56-96: LGTM: Well-structured interface definitions.All five new interfaces (
DecodedAuthzGrantMessage,DecodedAuthzRevokeMessage,DecodedFeegrantGrantAllowanceMessage,DecodedFeegrantRevokeAllowanceMessage) follow consistent patterns:
- Extend
DecodedMessageBaseappropriately- Use descriptive action names following the existing convention
- Use index signatures (
[key: string]: unknown) forauthorizationandallowanceobjects to allow flexibility for different types- Properly mark optional fields (like
expiration)src/tests/cosmos/move/authz-message-types.fixture.ts (4)
3-147: LGTM: Comprehensive delegate fixture.The
mockMsgExecWithDelegatefixture provides thorough test data including:
- Complete transaction structure with auth_info and body
- Detailed event logs with proper attributes (validator, delegator, amounts)
- Correct
authz_msg_indexattributes in events for tracking nested messages- Realistic gas values and timestamps
149-220: LGTM: Complete API response mocks.The
mockApiResponsesForDelegateprovides realistic mock responses for:
- Account resources with proper Move resource structures
- Metadata endpoint returning token information
- Validator endpoint with commission and description details
222-428: LGTM: Comprehensive withdraw reward and undelegate fixtures.Both
mockMsgExecWithWithdrawRewardand its API responses follow the same thorough pattern as the delegate fixture, covering:
- Proper event structure for reward withdrawal
- Correct coin transfer events
- Appropriate validator and account data
430-527: LGTM: Undelegate fixture with proper completion time.The
mockMsgExecWithUndelegatefixture correctly includes:
completion_timein the unbond event (Line 453)- Proper event structure
- Efficient reuse of API responses via
mockApiResponsesForUndelegate = mockApiResponsesForDelegate(Line 527)
- Add mock API response for burned NFT token address - Return empty resources array to simulate burned NFT state - Fixes test failure where metadata resolver queries burned token
…nt-support # Conflicts: # src/decoder-registry.ts # src/decoder.ts # src/interfaces/cosmos.ts # src/message-types.ts
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/decoders/cosmos/authz.ts (3)
26-29: Consider more descriptive error messages.The error message could include validation details to aid debugging. Consider using
parsed.errorto provide context.const parsed = zMsgExec.safeParse(message); if (!parsed.success) { - throw new Error("Invalid authz exec message"); + throw new Error(`Invalid authz exec message: ${parsed.error.message}`); }
92-95: Consider more descriptive error messages.Similar to the exec decoder, include validation details for easier debugging.
const parsed = zMsgGrant.safeParse(message); if (!parsed.success) { - throw new Error("Invalid authz grant message"); + throw new Error(`Invalid authz grant message: ${parsed.error.message}`); }
124-127: Consider more descriptive error messages.Include validation details to maintain consistency with the other decoders.
const parsed = zMsgRevoke.safeParse(message); if (!parsed.success) { - throw new Error("Invalid authz revoke message"); + throw new Error(`Invalid authz revoke message: ${parsed.error.message}`); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (5)
src/decoder.ts(3 hunks)src/decoders/cosmos/authz.ts(1 hunks)src/tests/cosmos/move/authz.fixture.ts(1 hunks)src/tests/cosmos/move/authz.test.ts(1 hunks)src/tests/cosmos/move/nft/nft-burn.fixture.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/tests/cosmos/move/authz.test.ts
- src/tests/cosmos/move/nft/nft-burn.fixture.ts
- src/decoder.ts
🧰 Additional context used
🪛 Gitleaks (8.28.0)
src/tests/cosmos/move/authz.fixture.ts
[high] 915-915: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 1438-1438: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 1930-1930: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (1)
src/tests/cosmos/move/authz.fixture.ts (1)
915-915: Static analysis false positive - safe to ignore.The flagged values are blockchain public keys used in transaction signatures, not secret API keys. Public keys are meant to be publicly visible and are part of the transaction data structure.
Also applies to: 1438-1438, 1930-1930
Poafs1
left a comment
Poafs1
left a comment
There was a problem hiding this comment.
Everything look good now, but let's wait for @evilpeach to re-review again.
|
@Poafs1 please help me as follows
|
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (1)
src/tests/cosmos/move/feegrant.fixture.ts (1)
242-329: Add source comment for transaction.While the transaction hash appears to be in a valid format, it's unclear whether this is sourced from a real transaction. Add a source comment (e.g.,
// Source: https://scan-api.initia.xyz/v1/...) to document the origin of this fixture, consistent with the pattern used inauthz.fixture.ts.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (4)
src/tests/cosmos/move/authz.fixture.ts(1 hunks)src/tests/cosmos/move/authz.test.ts(1 hunks)src/tests/cosmos/move/feegrant.fixture.ts(1 hunks)src/tests/cosmos/move/feegrant.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/tests/cosmos/move/authz.test.ts
🧰 Additional context used
🪛 Gitleaks (8.29.0)
src/tests/cosmos/move/authz.fixture.ts
[high] 485-485: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 977-977: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (3)
src/tests/cosmos/move/authz.fixture.ts (2)
485-485: Static analysis false positives.The static analysis tool flagged these lines as containing API keys, but these are actually public keys within test transaction data and pose no security risk. These warnings can be safely dismissed.
Also applies to: 977-977
1067-1199: Verify whether real transaction requirement applies to all fixtures or is context-specific.The review correctly identifies that
mockMsgExecWithMultipleuses a mock hash ("TEST789") while other fixtures use real sources. However, I could not find documented PR objectives stating that all fixtures must use real transactions. WhilemockRealMsgExecTransactionandmockRealMsgExecWithDifferentAddressesfollow the real transaction pattern,mockMsgExecWithMultiplemay intentionally use synthetic data since it tests a specific scenario with multiple inner messages that may not require a real blockchain transaction. Confirm whether using real transactions is a hard requirement for all fixtures or an acceptable approach for certain test cases.src/tests/cosmos/move/feegrant.test.ts (1)
1-138: LGTM! Well-structured test suite.The test file is cleanly organized with comprehensive coverage of feegrant message types, proper mocking setup, and clear assertions.
Add comprehensive decoder support for Cosmos SDK authz and feegrant modules.
Changes:
Summary by CodeRabbit
Release Notes
New Features
Documentation
Tests