Add TikTok as AuthProvider #154
Conversation
Introduces TikTok as a new AuthProvider across InAppWallet and EcosystemWallet, updating relevant enums, switch statements, and OAuth login link generation. Also refactors some code for consistency, fixes minor formatting, and ensures TikTok is handled in provider mappings and UI logic.
WalkthroughAdded AuthProvider.TikTok and its mapping in EcosystemWallet.Create; server-side OAuth path now lowercases provider when building headless OAuth links. Console sample updated (added usings; previously commented private-key wallet generation is now active; example comment switched to TikTok). Other edits are formatting-only. Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant EcosystemWallet
participant AuthServer
App->>EcosystemWallet: Create(authProvider=TikTok)
note right of EcosystemWallet #f0f8ff: Maps AuthProvider.TikTok → "TikTok"
EcosystemWallet->>AuthServer: FetchHeadlessOauthLoginLink("tiktok")
note right of AuthServer #f8f8ff: authProvider lowercased when building URI
AuthServer-->>EcosystemWallet: OAuth login URL
EcosystemWallet-->>App: Returns URL
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type 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 (3)
Thirdweb/Thirdweb.Wallets/SmartWallet/SmartWallet.cs (2)
56-98: Consider freezing the token-paymaster lookup to prevent accidental mutation
_tokenPaymasterConfigis intended to be a constant lookup table.
Declaring it as a mutableDictionary<,>leaves the door open for run-time mutation from external code (especially because the field isprivate static readonly, notprivate static).
If immutability is desired, switch toImmutableDictionaryor expose only a read-only view withAsReadOnly()to harden defensive coding.-private static readonly Dictionary<TokenPaymaster, TokenPaymasterConfig> _tokenPaymasterConfig = new() +private static readonly ImmutableDictionary<TokenPaymaster, TokenPaymasterConfig> _tokenPaymasterConfig = + new Dictionary<TokenPaymaster, TokenPaymasterConfig> + { + // … same payload … + }.ToImmutableDictionary();
70-96: Gitleaks warnings are false positives – no action requiredLines containing the on-chain addresses (
PaymasterAddress/TokenAddress) trigger “generic-api-key” hits from Gitleaks.
These are publicly-known contract addresses, not secrets. Documenting this once will help silence repeated security-review noise.Thirdweb/Thirdweb.Wallets/InAppWallet/EcosystemWallet/EcosystemWallet.cs (1)
842-846: Readable OTP pre-auth logicThe terse ternary chain hampers readability and debuggability.
An early-exitif/elseblock is clearer and avoids the hiddenExceptionside-effect inside the expression.- var serverRes = - string.IsNullOrEmpty(this.Email) && string.IsNullOrEmpty(this.PhoneNumber) ? throw new Exception("Email or Phone Number is required for OTP login") - : this.Email == null ? await this.EmbeddedWallet.VerifyPhoneOtpAsync(this.PhoneNumber, otp).ConfigureAwait(false) - : await this.EmbeddedWallet.VerifyEmailOtpAsync(this.Email, otp).ConfigureAwait(false); + Server.VerifyResult serverRes; + if (string.IsNullOrEmpty(this.Email) && string.IsNullOrEmpty(this.PhoneNumber)) + throw new Exception("Email or Phone Number is required for OTP login"); + + if (this.Email == null) + serverRes = await this.EmbeddedWallet.VerifyPhoneOtpAsync(this.PhoneNumber, otp).ConfigureAwait(false); + else + serverRes = await this.EmbeddedWallet.VerifyEmailOtpAsync(this.Email, otp).ConfigureAwait(false);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
Thirdweb.Console/Program.cs(1 hunks)Thirdweb.Tests/Thirdweb.Utils/Thirdweb.Utils.Tests.cs(9 hunks)Thirdweb/Thirdweb.RPC/ThirdwebRPC.cs(1 hunks)Thirdweb/Thirdweb.Utils/Utils.cs(4 hunks)Thirdweb/Thirdweb.Wallets/InAppWallet/EcosystemWallet/EcosystemWallet.cs(15 hunks)Thirdweb/Thirdweb.Wallets/InAppWallet/EmbeddedWallet.Authentication/Server.cs(5 hunks)Thirdweb/Thirdweb.Wallets/InAppWallet/InAppWallet.Types.cs(1 hunks)Thirdweb/Thirdweb.Wallets/SmartWallet/SmartWallet.cs(15 hunks)
🧰 Additional context used
🪛 Gitleaks (8.27.2)
Thirdweb/Thirdweb.Wallets/SmartWallet/SmartWallet.cs
74-74: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
84-84: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
94-94: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ 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-test-cov
🔇 Additional comments (15)
Thirdweb/Thirdweb.RPC/ThirdwebRPC.cs (1)
120-120: LGTM! Good formatting consistency.The trailing comma addition improves code maintainability and follows modern C# formatting conventions.
Thirdweb/Thirdweb.Wallets/InAppWallet/InAppWallet.Types.cs (1)
23-23: TikTok provider integration verifiedAll necessary references for the new
TikTokprovider are present:
- Thirdweb/Thirdweb.Wallets/InAppWallet/InAppWallet.Types.cs
•TikTokenum value added.- Thirdweb/Thirdweb.Wallets/InAppWallet/EcosystemWallet/EcosystemWallet.cs
•authProvider switchmapsThirdweb.AuthProvider.TikTok => "TikTok".
•case "TikTok":branch handles parsing of incoming provider strings.
•LoginWithOauthuses the mapped"TikTok"string against the generic OAuth endpoint.No additional provider-specific URLs or dictionaries are required—the generic login flow covers TikTok. Everything is wired up correctly.
Thirdweb.Console/Program.cs (1)
745-745: LGTM! Example code updated to demonstrate TikTok usage.The commented example code now shows how to use the new TikTok authentication provider, which serves as helpful documentation for developers.
Thirdweb.Tests/Thirdweb.Utils/Thirdweb.Utils.Tests.cs (1)
232-232: LGTM! Consistent formatting improvements.The trailing comma additions throughout the test methods improve code consistency and maintainability. This aligns with modern C# formatting conventions and makes future diffs cleaner.
Also applies to: 255-255, 276-276, 303-303, 320-320, 337-337, 354-354, 371-371, 388-388
Thirdweb/Thirdweb.Utils/Utils.cs (4)
715-715: LGTM - Formatting improvement for consistency.Adding the trailing comma in the list initialization improves code consistency and makes future additions easier.
1159-1159: LGTM - Formatting improvement for consistency.Adding the trailing comma in the object initializer improves code consistency and makes future additions easier.
1189-1189: LGTM - Formatting improvement for consistency.Adding the trailing comma in the object initializer improves code consistency and makes future additions easier.
1224-1224: LGTM - Formatting improvement for consistency.Adding the trailing comma in the anonymous object improves code consistency and makes future additions easier.
Thirdweb/Thirdweb.Wallets/InAppWallet/EmbeddedWallet.Authentication/Server.cs (5)
111-116: LGTM - Formatting improvement for consistency.The dictionary initialization with trailing commas improves code consistency and readability.
391-391: LGTM - Formatting improvement for consistency.Removing the trailing comma in the UriBuilder object initializer improves formatting consistency.
402-402: LGTM - Formatting improvement for consistency.Removing the trailing comma in the UriBuilder object initializer improves formatting consistency.
420-420: LGTM - Formatting improvement for consistency.Removing the trailing comma in the JsonSerializer object initializer improves formatting consistency.
203-203: Lowercase conversion for authProvider is consistent and safeAll existing OAuth‐link URLs in both EmbeddedWallet and EcosystemWallet already normalize the provider name to lowercase before sending to the server (e.g. FetchHeadlessOauthLoginLinkAsync, GenerateExternalLoginLink). Session resumption and storage still use the original casing for comparison, so this change only affects URL paths—matching the server’s lowercase routes—and will not break any existing providers. Proceed with approval.
Thirdweb/Thirdweb.Wallets/InAppWallet/EcosystemWallet/EcosystemWallet.cs (2)
140-143: Double-check server support for the newTikTokprovider string
"TikTok"is now surfaced in theAuthProvidermapping, but the backend must also:
- Accept the exact case-sensitive string
"TikTok"in/oauthendpoints.- Return
"TikTok"in user-status payloads, otherwise linking/unlinking will fail the switch-case comparisons later in this file.Please confirm with the auth service before shipping, or guard with a fallback / error message.
748-751: Keep the OAuth switch-case in sync with the mapping aboveThe
LinkAccountdispatcher already adds theTikTokcase – good.
Just ensure any future provider additions are updated in both places (mapping + dispatcher) to avoid unreachable code paths.
Updated the EIP-155 detection logic to first check if the Arachnid contract is deployed before sending a pre-155 transaction. Also added missing trailing commas in several object initializations for consistency.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #154 +/- ##
==========================================
- Coverage 56.85% 56.80% -0.05%
==========================================
Files 44 44
Lines 6992 6992
Branches 895 895
==========================================
- Hits 3975 3972 -3
- Misses 2829 2832 +3
Partials 188 188 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (4)
Thirdweb/Thirdweb.Wallets/InAppWallet/EmbeddedWallet.Authentication/AWS.cs (1)
138-140: Include x-amz-security-token in SigV4 canonical/signed headersTemporary AWS credentials (Cognito-issued/session tokens) require the
x-amz-security-tokenheader to be part of both the canonical request and theSignedHeaders. Omitting it will cause AWS to reject the signature.• File: Thirdweb/Thirdweb.Wallets/InAppWallet/EmbeddedWallet.Authentication/AWS.cs
Lines 138–140: buildcanonicalHeadersandsignedHeadersto conditionally includex-amz-security-tokenwhencredentials.SessionTokenis non-empty.Apply this diff:
--- a/Thirdweb/Thirdweb.Wallets/InAppWallet/EmbeddedWallet.Authentication/AWS.cs +++ b/Thirdweb/Thirdweb.Wallets/InAppWallet/EmbeddedWallet.Authentication/AWS.cs @@ -135,7 +135,18 @@ var amzDate = dateTimeNow.ToString(amzDateFormat); var dateStamp = dateTimeNow.ToString("yyyyMMdd"); - var canonicalHeaders = $"host:{new Uri(endpoint).Host}\n" + $"x-amz-date:{amzDate}\n"; - var signedHeaders = "host;x-amz-date"; + var canonicalHeadersBuilder = new StringBuilder(); + canonicalHeadersBuilder.Append($"host:{new Uri(endpoint).Host}\n"); + canonicalHeadersBuilder.Append($"x-amz-date:{amzDate}\n"); + if (!string.IsNullOrEmpty(credentials.SessionToken)) + { + canonicalHeadersBuilder.Append($"x-amz-security-token:{credentials.SessionToken}\n"); + } + var canonicalHeaders = canonicalHeadersBuilder.ToString(); + var signedHeaders = string.IsNullOrEmpty(credentials.SessionToken) + ? "host;x-amz-date" + : "host;x-amz-date;x-amz-security-token"; // ... @@ -163,7 +174,8 @@ client.AddHeader("x-amz-date", amzDate); client.AddHeader("Authorization", authorizationHeader); - if (!string.IsNullOrEmpty(credentials.SessionToken)) + // still add the header for the request itself + if (!string.IsNullOrEmpty(credentials.SessionToken)) { client.AddHeader("x-amz-security-token", credentials.SessionToken); }Thirdweb/Thirdweb.Bridge/ThirdwebBridge.Types.cs (1)
399-404: Map missing statuses inStatusData.StatusTypeCurrently, "PROCESSING" and "CREATED" resolve to UNKNOWN, which is inconsistent with the enum and with
OnrampStatusData. Add explicit mappings.this.Status switch { "FAILED" => StatusType.FAILED, "PENDING" => StatusType.PENDING, "COMPLETED" => StatusType.COMPLETED, "NOT_FOUND" => StatusType.NOT_FOUND, + "PROCESSING" => StatusType.PROCESSING, + "CREATED" => StatusType.CREATED, _ => StatusType.UNKNOWN, };Optional: add unit tests to cover all string-to-enum mappings for both StatusData and OnrampStatusData.
Thirdweb/Thirdweb.Extensions/ThirdwebExtensions.cs (1)
1783-1785: Bug: Wrong method invoked for ERC721 claim condition by ID.
DropERC721_GetActiveClaimConditionreturns ERC20 claim condition data instead of ERC721. This is a functional mismatch and will return the wrong schema/type.- var activeClaimConditionId = await contract.DropERC721_GetActiveClaimConditionId(); - return await contract.DropERC20_GetClaimConditionById(activeClaimConditionId); + var activeClaimConditionId = await contract.DropERC721_GetActiveClaimConditionId(); + return await contract.DropERC721_GetClaimConditionById(activeClaimConditionId);Consider adding a small test to catch this in the future (ensuring ERC721 path does not call ERC20 helpers).
Thirdweb/Thirdweb.Indexer/ThirdwebInsight.Extensions.cs (1)
14-21: Fix potential NullReference in switch default armLine 20 references token.Contract.Type without null-propagation while the switch input is token.Contract?.Type. If Contract is null, constructing the exception message will throw. Make it null-safe.
Apply this minimal diff:
- _ => throw new Exception($"Unknown NFT type: {token.Contract.Type}"), + _ => throw new Exception($"Unknown NFT type: {token.Contract?.Type ?? "null"}"),
🧹 Nitpick comments (8)
.editorconfig (1)
48-49: Collection initializer/expressions preferences set to falseConfirm this is intentional to prefer expanded add-calls and avoid C# 12 collection expressions. If so, consider also setting IDE0028/IDE0263 severities to avoid conflicting analyzer suggestions.
Example (only if you want to suppress these suggestions):
# Use collection initializers dotnet_diagnostic.IDE0028.severity = none # Use collection expression dotnet_diagnostic.IDE0263.severity = noneThirdweb/Thirdweb.Wallets/InAppWallet/EmbeddedWallet.Encryption/EmbeddedWallet.Cryptography.cs (1)
110-116: PBKDF2 derivation block unchanged; OKIndentation-only change. No issues spotted. Optional: consider Rfc2898DeriveBytes.Pbkdf2 (built-in) if you ever want to remove the BouncyCastle dependency here.
Thirdweb.Tests/Thirdweb.Wallets/Thirdweb.Wallets.Tests.cs (1)
105-105: Trailing comma in object initializerStyle-only; no effect on tests. LGTM.
To align with the PR goal (TikTok AuthProvider) and Codecov feedback, consider adding tests covering TikTok provider mapping and OAuth login link generation paths.
Thirdweb/Thirdweb.AI/ThirdwebNebula.cs (1)
100-108: Nit: Typo in variable namecontextFilerUse
contextFilterto match the method name and property, improving readability and searchability.- var contextFiler = await this.PrepareContextFilter(wallet, context); + var contextFilter = await this.PrepareContextFilter(wallet, context); ... - ContextFilter = contextFiler, + ContextFilter = contextFilter,Apply the same rename in all four methods where this local is used.
Also applies to: 123-131, 151-159, 185-193
Thirdweb.Tests/Thirdweb.Extensions/Thirdweb.Extensions.Tests.cs (2)
718-739: Simplify Assert.ThrowsAsync lambdas and keep style consistentUse
() => taskinstead ofasync () => await taskto reduce noise and align with the rest of the file.Example diffs (apply the same pattern to the other cases in these ranges):
- _ = await Assert.ThrowsAsync<ArgumentNullException>(async () => - await contract.ERC1155_SafeBatchTransferFrom(null, null, null, new BigInteger[] { validTokenId }, new BigInteger[] { validAmount }, validData) - ); + _ = await Assert.ThrowsAsync<ArgumentNullException>(() => + contract.ERC1155_SafeBatchTransferFrom(null, null, null, new BigInteger[] { validTokenId }, new BigInteger[] { validAmount }, validData) + ); - _ = await Assert.ThrowsAsync<ArgumentException>(async () => - await contract.ERC1155_SafeBatchTransferFrom(wallet, validAddress, validAddress, null, new BigInteger[] { validAmount }, validData) - ); + _ = await Assert.ThrowsAsync<ArgumentException>(() => + contract.ERC1155_SafeBatchTransferFrom(wallet, validAddress, validAddress, null, new BigInteger[] { validAmount }, validData) + );Optional minor style tweak in these lines: prefer type inference for small inline arrays for brevity:
- contract.ERC1155_SafeBatchTransferFrom(..., new BigInteger[] { validTokenId }, new BigInteger[] { validAmount }, ...) + contract.ERC1155_SafeBatchTransferFrom(..., new[] { validTokenId }, new[] { validAmount }, ...)Also applies to: 752-754, 764-766, 1657-1662
1425-1425: MintRequest inline initializations — LGTM; optional Uri nullness noteEdits look good. If the goal is to test default-filling more explicitly, consider using
Uri = nullrather than"". Not required; current assertions still validate defaults properly.Also applies to: 1533-1533, 1701-1701
Thirdweb/Thirdweb.Extensions/ThirdwebExtensions.cs (1)
110-112: Prefer guard clauses over nested ternaries; add null checks to avoid NREs.Current form is terse but harder to read and will NRE if
nftornft.Metadatais null. Recommend explicit checks and a null-conditional onImagefor safer behavior.- return client == null ? throw new ArgumentNullException(nameof(client)) - : string.IsNullOrEmpty(nft.Metadata.Image) ? Array.Empty<byte>() - : await ThirdwebStorage.Download<byte[]>(client, nft.Metadata.Image).ConfigureAwait(false); + if (client == null) throw new ArgumentNullException(nameof(client)); + if (nft == null) throw new ArgumentNullException(nameof(nft)); + + var image = nft.Metadata?.Image; + if (string.IsNullOrEmpty(image)) + return Array.Empty<byte>(); + + return await ThirdwebStorage.Download<byte[]>(client, image).ConfigureAwait(false);Thirdweb/Thirdweb.Wallets/PrivateKeyWallet/PrivateKeyWallet.cs (1)
361-362: Trailing commas in RLP list initializers: OKNo behavior change; improves diff stability.
If you want to boost coverage, I can add a test that ensures a transaction with AuthorizationList produces a type 0x04 prefixed raw tx (and 0x02 otherwise).
Also applies to: 470-471
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (48)
.csharpierrc(0 hunks).editorconfig(1 hunks)Directory.Build.props(1 hunks)Directory.Packages.props(1 hunks)Thirdweb.Console/Thirdweb.Console.csproj(1 hunks)Thirdweb.Tests/Thirdweb.Contracts/Thirdweb.Contracts.Tests.cs(4 hunks)Thirdweb.Tests/Thirdweb.Extensions/Thirdweb.Extensions.Tests.cs(10 hunks)Thirdweb.Tests/Thirdweb.Extensions/Thirdweb.MarketplaceExtensions.Tests.cs(4 hunks)Thirdweb.Tests/Thirdweb.Http/Thirdweb.Http.Tests.cs(3 hunks)Thirdweb.Tests/Thirdweb.Tests.csproj(1 hunks)Thirdweb.Tests/Thirdweb.Transactions/Thirdweb.Transactions.Tests.cs(4 hunks)Thirdweb.Tests/Thirdweb.Transactions/Thirdweb.ZkSmartWallet.Tests.cs(3 hunks)Thirdweb.Tests/Thirdweb.Wallets/Thirdweb.PrivateKeyWallet.Tests.cs(2 hunks)Thirdweb.Tests/Thirdweb.Wallets/Thirdweb.SmartWallet.Tests.cs(5 hunks)Thirdweb.Tests/Thirdweb.Wallets/Thirdweb.Wallets.Tests.cs(2 hunks)Thirdweb/Thirdweb.AI/ThirdwebNebula.cs(3 hunks)Thirdweb/Thirdweb.Bridge/ThirdwebBridge.Types.cs(4 hunks)Thirdweb/Thirdweb.Bridge/ThirdwebBridge.cs(6 hunks)Thirdweb/Thirdweb.Extensions/ThirdwebExtensions.Types.cs(1 hunks)Thirdweb/Thirdweb.Extensions/ThirdwebExtensions.cs(9 hunks)Thirdweb/Thirdweb.Extensions/ThirdwebMarketplaceExtensions.Types.cs(2 hunks)Thirdweb/Thirdweb.Indexer/ThirdwebInsight.Extensions.cs(2 hunks)Thirdweb/Thirdweb.Indexer/ThirdwebInsight.cs(2 hunks)Thirdweb/Thirdweb.Pay/ThirdwebPay.GetBuyHistory.cs(2 hunks)Thirdweb/Thirdweb.Pay/ThirdwebPay.GetBuyWithCryptoQuote.cs(1 hunks)Thirdweb/Thirdweb.Pay/ThirdwebPay.GetBuyWithCryptoStatus.cs(1 hunks)Thirdweb/Thirdweb.Pay/ThirdwebPay.GetBuyWithFiatCurrencies.cs(1 hunks)Thirdweb/Thirdweb.Pay/ThirdwebPay.GetBuyWithFiatQuote.cs(1 hunks)Thirdweb/Thirdweb.Pay/ThirdwebPay.GetBuyWithFiatStatus.cs(1 hunks)Thirdweb/Thirdweb.Pay/Types.GetBuyWithCryptoQuote.cs(1 hunks)Thirdweb/Thirdweb.Pay/Types.GetBuyWithCryptoStatus.cs(2 hunks)Thirdweb/Thirdweb.Pay/Types.GetBuyWithFiatQuote.cs(1 hunks)Thirdweb/Thirdweb.Pay/Types.Shared.cs(1 hunks)Thirdweb/Thirdweb.Transactions/ThirdwebTransaction.cs(2 hunks)Thirdweb/Thirdweb.Utils/Utils.Types.cs(1 hunks)Thirdweb/Thirdweb.Wallets/EIP712.cs(1 hunks)Thirdweb/Thirdweb.Wallets/EIP712Encoder.cs(2 hunks)Thirdweb/Thirdweb.Wallets/EngineWallet/EngineWallet.cs(3 hunks)Thirdweb/Thirdweb.Wallets/IThirdwebWallet.cs(1 hunks)Thirdweb/Thirdweb.Wallets/InAppWallet/EmbeddedWallet.Authentication/AWS.cs(1 hunks)Thirdweb/Thirdweb.Wallets/InAppWallet/EmbeddedWallet.Encryption/EmbeddedWallet.Cryptography.cs(2 hunks)Thirdweb/Thirdweb.Wallets/InAppWallet/EmbeddedWallet.Encryption/Secrets.cs(3 hunks)Thirdweb/Thirdweb.Wallets/InAppWallet/EmbeddedWallet.Exceptions/VerificationException.cs(1 hunks)Thirdweb/Thirdweb.Wallets/InAppWallet/EmbeddedWallet/EmbeddedWallet.AccountLinking.cs(1 hunks)Thirdweb/Thirdweb.Wallets/PrivateKeyWallet/PrivateKeyWallet.cs(2 hunks)Thirdweb/Thirdweb.Wallets/SmartWallet/Thirdweb.AccountAbstraction/AATypes.cs(1 hunks)Thirdweb/Thirdweb.Wallets/SmartWallet/Thirdweb.AccountAbstraction/BundlerClient.cs(1 hunks)Thirdweb/Thirdweb.csproj(1 hunks)
💤 Files with no reviewable changes (1)
- .csharpierrc
✅ Files skipped from review due to trivial changes (32)
- Thirdweb/Thirdweb.Pay/Types.Shared.cs
- Directory.Build.props
- Directory.Packages.props
- Thirdweb/Thirdweb.Pay/ThirdwebPay.GetBuyWithCryptoStatus.cs
- Thirdweb.Tests/Thirdweb.Http/Thirdweb.Http.Tests.cs
- Thirdweb/Thirdweb.Pay/Types.GetBuyWithCryptoQuote.cs
- Thirdweb.Console/Thirdweb.Console.csproj
- Thirdweb/Thirdweb.Wallets/InAppWallet/EmbeddedWallet/EmbeddedWallet.AccountLinking.cs
- Thirdweb/Thirdweb.Wallets/SmartWallet/Thirdweb.AccountAbstraction/AATypes.cs
- Thirdweb/Thirdweb.Pay/ThirdwebPay.GetBuyHistory.cs
- Thirdweb/Thirdweb.Pay/ThirdwebPay.GetBuyWithCryptoQuote.cs
- Thirdweb/Thirdweb.Utils/Utils.Types.cs
- Thirdweb/Thirdweb.Pay/ThirdwebPay.GetBuyWithFiatQuote.cs
- Thirdweb/Thirdweb.Wallets/SmartWallet/Thirdweb.AccountAbstraction/BundlerClient.cs
- Thirdweb.Tests/Thirdweb.Extensions/Thirdweb.MarketplaceExtensions.Tests.cs
- Thirdweb/Thirdweb.csproj
- Thirdweb.Tests/Thirdweb.Transactions/Thirdweb.ZkSmartWallet.Tests.cs
- Thirdweb.Tests/Thirdweb.Contracts/Thirdweb.Contracts.Tests.cs
- Thirdweb/Thirdweb.Pay/ThirdwebPay.GetBuyWithFiatStatus.cs
- Thirdweb/Thirdweb.Pay/Types.GetBuyWithFiatQuote.cs
- Thirdweb/Thirdweb.Wallets/EIP712Encoder.cs
- Thirdweb/Thirdweb.Pay/ThirdwebPay.GetBuyWithFiatCurrencies.cs
- Thirdweb/Thirdweb.Extensions/ThirdwebMarketplaceExtensions.Types.cs
- Thirdweb/Thirdweb.Extensions/ThirdwebExtensions.Types.cs
- Thirdweb/Thirdweb.Wallets/IThirdwebWallet.cs
- Thirdweb/Thirdweb.Wallets/EngineWallet/EngineWallet.cs
- Thirdweb/Thirdweb.Indexer/ThirdwebInsight.cs
- Thirdweb.Tests/Thirdweb.Tests.csproj
- Thirdweb.Tests/Thirdweb.Wallets/Thirdweb.SmartWallet.Tests.cs
- Thirdweb/Thirdweb.Bridge/ThirdwebBridge.cs
- Thirdweb/Thirdweb.Pay/Types.GetBuyWithCryptoStatus.cs
- Thirdweb/Thirdweb.Wallets/InAppWallet/EmbeddedWallet.Exceptions/VerificationException.cs
🧰 Additional context used
🧬 Code Graph Analysis (3)
Thirdweb/Thirdweb.Wallets/InAppWallet/EmbeddedWallet.Encryption/EmbeddedWallet.Cryptography.cs (1)
Thirdweb/Thirdweb.Wallets/InAppWallet/EmbeddedWallet.Encryption/Secrets.cs (1)
GetBytes(36-40)
Thirdweb/Thirdweb.Wallets/PrivateKeyWallet/PrivateKeyWallet.cs (1)
Thirdweb/Thirdweb.Utils/Utils.cs (3)
HexToBytes(131-134)TrimZeroes(1095-1112)ToByteArrayForRLPEncoding(1201-1209)
Thirdweb/Thirdweb.Extensions/ThirdwebExtensions.cs (1)
Thirdweb/Thirdweb.Storage/ThirdwebStorage.cs (1)
ThirdwebStorage(8-106)
⏰ 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-test-cov
🔇 Additional comments (28)
Thirdweb/Thirdweb.Wallets/InAppWallet/EmbeddedWallet.Authentication/AWS.cs (1)
49-53: LGTM: trailing comma is harmless and improves diff stabilityNo functional change; consistent with the PR’s formatting cleanups.
.editorconfig (4)
2-2: Header comment tweak is fineNo functional impact.
8-9: Section header clarity is fineConsistent with the rest of the file; no issues.
18-18: .NET Coding Conventions header formattingLooks good; matches surrounding style.
11-16: Cross-platform EOL normalization confirmed in .gitattributesThe repository’s
.gitattributesalready includes:* text=autowhich instructs Git to normalize all text files (including C#) to LF in the repo and convert to the platform’s native EOL on checkout. This satisfies the cross-platform line ending workflow and prevents CRLF churn across contributors.
Thirdweb/Thirdweb.Wallets/EIP712.cs (1)
489-491: Trailing comma is harmless; behavior unchangedRLP empty list 0xc0 remains intact. No functional impact in SerializeEip712.
Thirdweb/Thirdweb.Wallets/InAppWallet/EmbeddedWallet.Encryption/Secrets.cs (2)
14-14: Formatting-only change to _nybblesNo behavior change. LGTM.
454-454: PrimitivePolynomialCoefficients formattingTrailing-comma removal only; constants unchanged. LGTM.
Thirdweb.Tests/Thirdweb.Wallets/Thirdweb.Wallets.Tests.cs (1)
224-230: Assert formatting approved & TikTok integration verified
- Assert formatting remains concise and readable—no behavior changes.
- TikTok is included in
AuthProvider(InAppWallet.Types.cs), mapped to"TikTok"inEcosystemWallet(string mappings and switch cases), and correctly handled byFetchHeadlessOauthLoginLinkAsync(EmbeddedWallet.Authentication/Server.cs).All required wiring is present. No further changes needed.
Thirdweb/Thirdweb.AI/ThirdwebNebula.cs (3)
9-9: Trailing comma in enum member — OKNo behavioral change; improves diff-friendliness.
85-86: Trailing comma in object initializer — OKPurely stylistic; no runtime impact.
224-229: Trailing comma in object initializer — OKConsistent with project-wide formatting changes.
Thirdweb/Thirdweb.Bridge/ThirdwebBridge.Types.cs (3)
384-385: Trailing comma in enumStatusType— OKStyle-only change; no effect on enums.
499-500: Trailing comma in enumOnrampProvider— OKNo functional change.
588-589: Trailing comma in switch expression — OKKeeps diffs minimal for future arms.
Thirdweb.Tests/Thirdweb.Extensions/Thirdweb.Extensions.Tests.cs (1)
1378-1378: Trailing commas in multiline initializers — LGTMThese commas improve future diffs and maintain consistency in the test suite.
Also applies to: 1480-1480, 1640-1640
Thirdweb/Thirdweb.Extensions/ThirdwebExtensions.cs (8)
1239-1240: LGTM: initializer trailing comma and defaults.Setting
QuantityOwned = 1for ERC721 and adding a trailing comma are fine for clarity and stable diffs.
1273-1273: LGTM: trailing comma in with-expression.No logic change; consistent with repo formatting standardization.
1589-1596: LGTM: trailing comma in fnArgs array (DropERC20_Claim).Purely stylistic; no behavior change.
1726-1734: LGTM: trailing comma in fnArgs array (DropERC721_Claim).Consistent formatting; no functional impact.
1890-1898: LGTM: trailing comma in fnArgs array (DropERC1155_Claim).Style-only change; looks good.
2080-2090: LGTM: trailing comma in TokenERC20_MintRequest initializer.Keeps diffs cleaner; no logic change.
2325-2337: LGTM: trailing comma in TokenERC721_MintRequest initializer.Formatting-only; all good.
2670-2684: LGTM: trailing comma in TokenERC1155_MintRequest initializer.Consistent with the rest of the PR’s formatting sweep.
Thirdweb/Thirdweb.Indexer/ThirdwebInsight.Extensions.cs (1)
34-35: Stylistic trailing comma is fineThe trailing comma in the object initializer improves diffs and consistency. No functional change.
Thirdweb.Tests/Thirdweb.Wallets/Thirdweb.PrivateKeyWallet.Tests.cs (1)
273-274: Trailing commas in object initializers are OKFormatting-only change. Valid in C#, no behavior impact.
Also applies to: 291-293
Thirdweb.Tests/Thirdweb.Transactions/Thirdweb.Transactions.Tests.cs (1)
14-14: LGTM: initializer comma cleanupsPure formatting for consistency; no functional change.
Also applies to: 168-168, 332-332, 369-369
Thirdweb/Thirdweb.Transactions/ThirdwebTransaction.cs (1)
265-268: Style-only changes acknowledged
- Divider ternary formatting: same logic, clearer layout.
- Trailing comma in zkSync transaction initializer: fine.
Also applies to: 493-494
Thirdweb/Thirdweb.Wallets/InAppWallet/EmbeddedWallet.Encryption/EmbeddedWallet.Cryptography.cs
Outdated
Show resolved
Hide resolved
Thirdweb/Thirdweb.Wallets/InAppWallet/EmbeddedWallet.Encryption/Secrets.cs
Outdated
Show resolved
Hide resolved
Updated the OAuth login URL construction in EcosystemWallet to use the lowercase value of AuthProvider. Also uncommented private key wallet generation in the console program for testing.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
Thirdweb.Console/Program.cs (2)
19-20: Generate vs. persist: prefer LoadOrGenerate and show the addressUncommenting generation changes runtime behavior by creating a new key each run. For a smoother dev experience, consider loading an existing key when present and printing the address so the section is demonstrably useful.
Apply this minimal change within the current lines to reuse existing keys:
- var privateKeyWallet = await PrivateKeyWallet.Generate(client); + var privateKeyWallet = await PrivateKeyWallet.LoadOrGenerate(client);Optionally, make the sample visible by printing the address (outside the current range):
-// var walletAddress = await privateKeyWallet.GetAddress(); -// Console.WriteLine($"PK Wallet address: {walletAddress}"); +var walletAddress = await privateKeyWallet.GetAddress(); +Console.WriteLine($"PK Wallet address: {walletAddress}");If you want persistence across runs when first generated, you could also save it once (optional):
await privateKeyWallet.Save();
749-749: Add dashboard prerequisite note for TikTok OAuthVerified that
AuthProvider.TikTokis defined and referenced (e.g. in EcosystemWallet.cs:140)- Headless OAuth links lowercase the provider internally (
FetchHeadlessOauthLoginLinkAsyncin Server.cs usesauthProvider.ToLower(), andGenerateExternalLoginLinkapplies.ToLower())Please add a one-liner above the commented sample in
Thirdweb.Console/Program.cs(around line 749):#region InAppWallet - OAuth + // NOTE: TikTok OAuth must be enabled in your thirdweb dashboard for this app/ecosystem. // var inAppWalletOAuth = await InAppWallet.Create(client: client, authProvider: AuthProvider.TikTok); // if (!await inAppWalletOAuth.IsConnected()) // {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
Thirdweb.Console/Program.cs(3 hunks)Thirdweb/Thirdweb.Wallets/InAppWallet/EcosystemWallet/EcosystemWallet.cs(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- Thirdweb/Thirdweb.Wallets/InAppWallet/EcosystemWallet/EcosystemWallet.cs
🧰 Additional context used
🧬 Code graph analysis (1)
Thirdweb.Console/Program.cs (1)
Thirdweb/Thirdweb.Wallets/PrivateKeyWallet/PrivateKeyWallet.cs (2)
PrivateKeyWallet(14-487)PrivateKeyWallet(24-28)
⏰ 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-test-cov
🔇 Additional comments (1)
Thirdweb.Console/Program.cs (1)
4-6: LGTM: imports added for browser launch and JSON serialization
System.DiagnosticsenablesProcessStartInfoused in OAuth samples, andNewtonsoft.Jsonmatches the serialization calls below. No concerns here.
Introduces TikTok as a new AuthProvider across InAppWallet and EcosystemWallet, updating relevant enums, switch statements, and OAuth login link generation. Also refactors some code for consistency, fixes minor formatting, and ensures TikTok is handled in provider mappings and UI logic.
PR-Codex overview
This PR focuses on enhancing the
InAppWalletfunctionality by adding support for theTikTokauthentication provider and adjusting the handling of authentication provider names to be case-insensitive.Detailed summary
TikTokto the list of authentication providers inInAppWallet.Types.cs.FetchHeadlessOauthLoginLinkAsyncinServer.csto useauthProvider.ToLower().Program.csto reflect the use ofTikTok.TikTokin account linking options inEcosystemWallet.cs.authProvider.ToLower()inLoginWithOtp.Summary by CodeRabbit
New Features
Bug Fixes
Style