Add context.log support for contractRegister#624
Conversation
WalkthroughThis change deletes the old contract registration context implementation, adds a proxied context for contract registration that includes a Changes
Sequence Diagram(s)sequenceDiagram
participant GlobalState
participant ChainFetcher
participant UserContext
participant ContractRegisterHandler
GlobalState->>ChainFetcher: runContractRegistersOrThrow(~reversedWithContractRegister, ~config)
ChainFetcher->>UserContext: getContractRegisterArgs(~onRegister, ~config)
UserContext-->>ChainFetcher: contractRegisterArgs (includes context with log)
ChainFetcher->>ContractRegisterHandler: contractRegister(contractRegisterArgs)
ContractRegisterHandler->>ContractRegisterHandler: context.log.info()
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes detected. Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used🧠 Learnings (1)scenarios/test_codegen/test/EventHandler_test.ts (1)⏰ 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). (1)
🔇 Additional comments (2)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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 (1)
codegenerator/cli/templates/static/codegen/src/Config.res (1)
140-146: Consider contract name uniqueness across chains.The mapping logic assumes contract names are unique across all chains. If multiple chains have contracts with the same name, later entries will overwrite earlier ones in the dictionary.
Consider implementing a more robust mapping strategy if contract name conflicts are possible:
- let addContractNameToContractNameMapping = Js.Dict.empty() - chains->Array.forEach(chainConfig => { - chainConfig.contracts->Array.forEach(contract => { - let addKey = "add" ++ contract.name->Utils.String.capitalize - addContractNameToContractNameMapping->Js.Dict.set(addKey, contract.name) - }) - }) + let addContractNameToContractNameMapping = Js.Dict.empty() + chains->Array.forEach(chainConfig => { + chainConfig.contracts->Array.forEach(contract => { + let addKey = "add" ++ contract.name->Utils.String.capitalize + // Only set if not already present, or consider chain-specific keys + if !addContractNameToContractNameMapping->Js.Dict.get(addKey)->Option.isSome { + addContractNameToContractNameMapping->Js.Dict.set(addKey, contract.name) + } + }) + })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
codegenerator/cli/templates/dynamic/codegen/src/ContextEnv.res.hbs(0 hunks)codegenerator/cli/templates/dynamic/codegen/src/TestHelpers_MockDb.res.hbs(1 hunks)codegenerator/cli/templates/dynamic/codegen/src/Types.res.hbs(1 hunks)codegenerator/cli/templates/static/codegen/src/Config.res(3 hunks)codegenerator/cli/templates/static/codegen/src/UserContext.res(1 hunks)codegenerator/cli/templates/static/codegen/src/eventFetching/ChainFetcher.res(2 hunks)codegenerator/cli/templates/static/codegen/src/globalState/GlobalState.res(1 hunks)scenarios/test_codegen/src/EventHandlers.ts(2 hunks)
💤 Files with no reviewable changes (1)
- codegenerator/cli/templates/dynamic/codegen/src/ContextEnv.res.hbs
🧰 Additional context used
🧠 Learnings (1)
scenarios/test_codegen/src/EventHandlers.ts (1)
Learnt from: JonoPrest
PR: enviodev/hyperindex#555
File: scenarios/test_codegen/test/lib_tests/EntityHistory_test.res:631-659
Timestamp: 2025-05-27T17:07:12.878Z
Learning: In isolated tests for EntityHistory rollbacks with controlled mock data setup, the ordering of getRollbackDiff results is deterministic and doesn't require sorting before comparison with deepStrictEqual.
🧬 Code Graph Analysis (1)
scenarios/test_codegen/src/EventHandlers.ts (1)
codegenerator/cli/npm/envio/src/Types.ts (1)
Logger(5-22)
⏰ 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). (1)
- GitHub Check: build_and_test
🔇 Additional comments (13)
codegenerator/cli/templates/dynamic/codegen/src/Types.res.hbs (1)
9-9: LGTM: Logging field addition is correct.The addition of the
logfield with typeEnvio.loggerproperly integrates logging capability into the contract registration context.codegenerator/cli/templates/dynamic/codegen/src/TestHelpers_MockDb.res.hbs (1)
374-374: ConfirmedrunContractRegistersOrThrowSignature Requires~configThe call in TestHelpers_MockDb.res.hbs correctly passes the
~configargument, matching the definition in ChainFetcher.res:let runContractRegistersOrThrow = async ( ~reversedWithContractRegister: array<Internal.eventItem>, ~config: Config.t, … ) => { … }No changes needed. Approving this addition.
scenarios/test_codegen/src/EventHandlers.ts (2)
2-2: LGTM: Logger import addition is appropriate.Adding
Loggerto the imports is necessary for the type assertion used in contract registration testing.
412-413: LGTM: Type assertion validates logging integration.The type assertion confirms that
context.logis properly typed asLoggerin the contract registration context, ensuring type safety for the new logging functionality.codegenerator/cli/templates/static/codegen/src/globalState/GlobalState.res (1)
577-577: LGTM: Config parameter addition maintains consistency.The addition of
~config=state.configparameter ensures contract registration handlers have access to configuration data, including the new contract name mapping. This is consistent with the similar change in TestHelpers.codegenerator/cli/templates/static/codegen/src/Config.res (2)
120-120: LGTM: Contract name mapping field addition.The new field
addContractNameToContractNameMappingproperly supports dynamic contract registration functionality.
163-163: LGTM: Field inclusion in config record.The contract name mapping is properly included in the configuration record.
codegenerator/cli/templates/static/codegen/src/eventFetching/ChainFetcher.res (2)
313-313: Function signature updated to include configuration parameter.The addition of the
~config: Config.tparameter aligns with the new contract register context implementation that requires configuration data for contract name mapping and address validation.
358-358: Function call updated to use the new UserContext implementation.The change from
ContextEnv.getContractRegisterArgstoUserContext.getContractRegisterArgswith the added~configparameter is consistent with the refactoring to move contract register context creation to a static implementation.codegenerator/cli/templates/static/codegen/src/UserContext.res (4)
334-342: Well-structured type definition for contract register parameters.The
contractRegisterParamstype properly encapsulates the necessary data for contract registration: event item, registration callback, and configuration. The callback function signature is well-typed with all required parameters.
344-376: Robust proxy traps implementation with proper validation.The proxy traps implementation includes several good practices:
- Proper logging support through the
logproperty- Efficient contract name lookup using pre-built mapping
- EVM address validation for address integrity
- Comprehensive error handling for invalid property access
The implementation follows the established patterns in the codebase.
378-395: Clean helper functions following established patterns.The
getContractRegisterContextandgetContractRegisterArgsfunctions follow the same patterns as other context creation functions in the file, ensuring consistency across the codebase. The function signatures are well-typed and the implementation is straightforward.
356-360: Address validation ensures data integrity.The EVM address validation using
Address.Evm.fromAddressOrThrowis a good security practice that ensures only valid addresses are registered for EVM ecosystems, preventing potential issues downstream.
2 participants
Closes #620
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Style
Tests