refactor: migrate publishIntent(s) function#552
Conversation
|
Warning Rate limit exceeded@cawabunga-bytes has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 29 minutes and 1 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThe changes refactor the codebase to remove direct usage of legacy Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Feature Module
participant SolverRelay as solverRelay (internal-utils)
participant Adapter as Legacy Adapter
Caller->>SolverRelay: publishIntent(s)(...)
SolverRelay-->>Caller: Result (new format)
Caller->>Adapter: convertPublishIntent(s)ToLegacyFormat(Result)
Adapter-->>Caller: Result (legacy format)
Estimated code review effort4 (~90 minutes) Poem
✨ 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: 3
🔭 Outside diff range comments (1)
src/sdk/solverRelay/utils/parseFailedPublishError.ts (1)
4-17: Add missing RELAY_PUBLISH_NETWORK_ERROR to ParsedPublishErrors typeThe
ParsedPublishErrorstype is missingRELAY_PUBLISH_NETWORK_ERRORwhich exists inPublishIntentsErrin the other file. This creates an inconsistency.export type ParsedPublishErrors = | { reason: | "RELAY_PUBLISH_SIGNATURE_EXPIRED" | "RELAY_PUBLISH_INTERNAL_ERROR" | "RELAY_PUBLISH_SIGNATURE_INVALID" | "RELAY_PUBLISH_NONCE_USED" | "RELAY_PUBLISH_INSUFFICIENT_BALANCE" | "RELAY_PUBLISH_PUBLIC_NOT_EXIST" + | "RELAY_PUBLISH_NETWORK_ERROR" } | { reason: "RELAY_PUBLISH_UNKNOWN_ERROR" serverReason: string }
🧹 Nitpick comments (1)
src/sdk/solverRelay/publishIntents.ts (1)
10-54: Consider simplifying the return typeSince this function performs synchronous conversion, returning a Promise is unnecessary and adds complexity.
-export function convertPublishIntentsToLegacyFormat( - result: Result< - solverRelay.PublishIntentsReturnType, - solverRelay.PublishIntentsErrorType - > -): Promise<Result<PublishIntentsOk, PublishIntentsErr>> { +export function convertPublishIntentsToLegacyFormat( + result: Result< + solverRelay.PublishIntentsReturnType, + solverRelay.PublishIntentsErrorType + > +): Result<PublishIntentsOk, PublishIntentsErr> { if (result.isOk()) { - return Promise.resolve(Ok(result.unwrap())) + return Ok(result.unwrap()) } const error = result.unwrapErr() const errorCode = error.code // Map new PublishErrorCode to old ParsedPublishErrors format let reason: ParsedPublishErrors["reason"] switch (errorCode) { // ... cases ... } - return Promise.resolve( - Err({ - reason, - serverReason: errorCode, - }) - ) + return Err({ + reason, + serverReason: errorCode, + })Note: This would require updating the usage in
otcMakerOrderCancellationActor.tsto remove the.then()call.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/features/gift/actors/giftMakerPublishingActor.ts(2 hunks)src/features/gift/actors/shared/giftClaimActor.ts(2 hunks)src/features/machines/swapIntentMachine.ts(2 hunks)src/features/otcDesk/actors/otcMakerOrderCancellationActor.ts(2 hunks)src/features/otcDesk/hooks/useOtcTakerConfirmTrade.ts(3 hunks)src/features/tokenMigration/machines/tokenMigrationMachine.ts(2 hunks)src/sdk/solverRelay/publishIntent.ts(0 hunks)src/sdk/solverRelay/publishIntents.ts(1 hunks)src/sdk/solverRelay/solverRelayHttpClient/apis.ts(0 hunks)src/sdk/solverRelay/utils/parseFailedPublishError.ts(2 hunks)
💤 Files with no reviewable changes (2)
- src/sdk/solverRelay/solverRelayHttpClient/apis.ts
- src/sdk/solverRelay/publishIntent.ts
🔇 Additional comments (12)
src/features/machines/swapIntentMachine.ts (2)
2-2: LGTM: Clean import additions for the migration.The imports properly bring in the new
solverRelayclient and the legacy format conversion utility.Also applies to: 8-8
267-270: LGTM: Proper migration to new solverRelay client.The change correctly replaces the direct
publishIntentcall withsolverRelay.publishIntentfollowed by legacy format conversion. The parameter signature matches and the promise chaining ensures proper error propagation.src/features/otcDesk/hooks/useOtcTakerConfirmTrade.ts (2)
1-1: LGTM: Consistent import changes for the migration.The imports correctly add the new
solverRelayclient and replace the oldpublishIntentsimport with the legacy conversion function.Also applies to: 12-12
99-104: LGTM: Proper publishIntents migration with legacy compatibility.The migration correctly replaces the direct
publishIntentscall withsolverRelay.publishIntentsand applies legacy format conversion. The parameter structure is maintained properly.src/features/gift/actors/shared/giftClaimActor.ts (2)
1-1: LGTM: Consistent import updates for migration.The imports properly add the new
solverRelayclient and replace the legacypublishIntentsimport with the conversion utility.Also applies to: 8-8
118-123: LGTM: Proper publishIntents migration in gift claim flow.The migration correctly updates the
publishGiftActorto usesolverRelay.publishIntentswith legacy format conversion. The empty quote_hashes array is appropriate for gift claiming scenarios.src/features/gift/actors/giftMakerPublishingActor.ts (2)
1-1: LGTM: Consistent import changes for migration.The imports properly add the new
solverRelayclient and the legacy conversion utility.Also applies to: 6-6
45-58: LGTM: Well-structured publishIntents migration with inline error handling.The migration correctly uses
solverRelay.publishIntentswith legacy format conversion. The inline error handling and result transformation logic maintains the expected tagged response format while properly handling both success and error cases.src/features/tokenMigration/machines/tokenMigrationMachine.ts (2)
1-1: LGTM: Proper import updates for migration.The imports correctly add the new
solverRelayclient and the legacy format conversion utility.Also applies to: 7-7
59-63: LGTM: Excellent type-safe migration of publishIntent actor.The migration uses proper TypeScript typing with
Parameters<typeof solverRelay.publishIntent>and correctly applies the spread operator to pass parameters. The legacy format conversion maintains backward compatibility.src/features/otcDesk/actors/otcMakerOrderCancellationActor.ts (2)
1-1: Import changes look goodThe imports correctly reference the new
solverRelayfrom@defuse-protocol/internal-utilsand the new conversion function.Also applies to: 9-9
69-83: Publisher actor migration implemented correctlyThe migration properly:
- Uses the new
solverRelay.publishIntentsAPI- Converts the result to legacy format for backward compatibility
- Preserves the existing error handling flow
| reason = "RELAY_PUBLISH_INSUFFICIENT_BALANCE" | ||
| break | ||
| case "PUBLIC_KEY_NOT_EXIST": | ||
| reason = "RELAY_PUBLISH_PUBLIC_NOT_EXIST" |
There was a problem hiding this comment.
Fix typo in error reason
The reason should be RELAY_PUBLISH_PUBLIC_KEY_NOT_EXIST to match the pattern and be more descriptive.
- reason = "RELAY_PUBLISH_PUBLIC_NOT_EXIST"
+ reason = "RELAY_PUBLISH_PUBLIC_KEY_NOT_EXIST"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| reason = "RELAY_PUBLISH_PUBLIC_NOT_EXIST" | |
| reason = "RELAY_PUBLISH_PUBLIC_KEY_NOT_EXIST" |
🤖 Prompt for AI Agents
In src/sdk/solverRelay/publishIntents.ts at line 42, the error reason string is
incorrectly set to "RELAY_PUBLISH_PUBLIC_NOT_EXIST". Update this string to
"RELAY_PUBLISH_PUBLIC_KEY_NOT_EXIST" to correct the typo and maintain
consistency with the naming pattern.
| reason = "RELAY_PUBLISH_INSUFFICIENT_BALANCE" | ||
| break | ||
| case "PUBLIC_KEY_NOT_EXIST": | ||
| reason = "RELAY_PUBLISH_PUBLIC_NOT_EXIST" |
There was a problem hiding this comment.
Fix typo in error reason (same as in publishIntents.ts)
- reason = "RELAY_PUBLISH_PUBLIC_NOT_EXIST"
+ reason = "RELAY_PUBLISH_PUBLIC_KEY_NOT_EXIST"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| reason = "RELAY_PUBLISH_PUBLIC_NOT_EXIST" | |
| reason = "RELAY_PUBLISH_PUBLIC_KEY_NOT_EXIST" |
🤖 Prompt for AI Agents
In src/sdk/solverRelay/utils/parseFailedPublishError.ts at line 57, there is a
typo in the error reason string "RELAY_PUBLISH_PUBLIC_NOT_EXIST". Correct this
typo to match the exact error reason used in publishIntents.ts for consistency
and correctness.
| reason = "RELAY_PUBLISH_UNKNOWN_ERROR" | ||
| break | ||
| case "NETWORK_ERROR": | ||
| reason = "RELAY_PUBLISH_UNKNOWN_ERROR" | ||
| break | ||
| default: | ||
| reason = "RELAY_PUBLISH_UNKNOWN_ERROR" | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error mapping to preserve information
NETWORK_ERRORshould map to the existingRELAY_PUBLISH_NETWORK_ERRORreason instead ofUNKNOWN_ERROR- The default case should include
serverReasonto help with debugging unmapped error codes
case "PUBLIC_KEY_NOT_EXIST":
reason = "RELAY_PUBLISH_PUBLIC_NOT_EXIST"
break
case "UNKNOWN_ERROR":
reason = "RELAY_PUBLISH_UNKNOWN_ERROR"
break
case "NETWORK_ERROR":
- reason = "RELAY_PUBLISH_UNKNOWN_ERROR"
+ reason = "RELAY_PUBLISH_NETWORK_ERROR"
break
default:
reason = "RELAY_PUBLISH_UNKNOWN_ERROR"
}
return {
tag: "err",
value:
- reason === "RELAY_PUBLISH_UNKNOWN_ERROR"
+ reason === "RELAY_PUBLISH_UNKNOWN_ERROR" || errorCode === "UNKNOWN_ERROR" || !["SIGNATURE_EXPIRED", "INTERNAL_ERROR", "SIGNATURE_INVALID", "NONCE_USED", "INSUFFICIENT_BALANCE", "PUBLIC_KEY_NOT_EXIST", "NETWORK_ERROR"].includes(errorCode)
? { reason, serverReason: errorCode }
: { reason },
}Note: You'll need to add RELAY_PUBLISH_NETWORK_ERROR to the ParsedPublishErrors type union if it doesn't already exist.
Also applies to: 72-74
🤖 Prompt for AI Agents
In src/sdk/solverRelay/utils/parseFailedPublishError.ts around lines 59 to 67,
update the error mapping so that the "NETWORK_ERROR" case assigns reason to
"RELAY_PUBLISH_NETWORK_ERROR" instead of "RELAY_PUBLISH_UNKNOWN_ERROR". Modify
the default case to include the serverReason value in the reason to aid
debugging of unmapped errors. Also ensure that RELAY_PUBLISH_NETWORK_ERROR is
added to the ParsedPublishErrors type union if it is not already present. Apply
the same changes to lines 72-74 as well.
* beta: refactor: migrate runtime and waitForIntentSettlement utils (#553) refactor: migrate failover and request utils (#554) refactor: migrate publishIntent(s) function (#552) refactor: migrate logger from internal-utils (#551) refactor: migrate poaBridge utils (#550) refactor: use toError and NEP-141 storage utils from internal-utils (#548)
|
🎉 This PR is included in version 1.0.0-beta.204 🎉 The release is available on: Your semantic-release bot 📦🚀 |
3 participants
Summary by CodeRabbit