Add ThirdwebContract.Deploy via new Thirdweb.Api #155
Conversation
Adds the auto-generated Thirdweb.Api/GeneratedClient.cs (https://api.thirdweb.com/reference integration) and supporting nswag.json. Updates .editorconfig to exclude generated API files from linting. Removes Directory.Build.props, Directory.Packages.props, and Makefile. Updates project files to specify explicit package versions and build settings. Refactors and cleans up test and console code for consistency and style.
WalkthroughIntroduces OpenAPI client generation (nswag.json) and a Windows helper script (tw.bat); adds an HttpClient adapter and wires an internal ThirdwebApiClient into ThirdwebClient; adds contract deployment and transaction-hash waiting helpers; moves console samples toward server-wallet flows; deletes centralized MSBuild and package props and a CSharpier config; pins project package versions; applies numerous formatting-only edits. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App as App Code
participant TC as ThirdwebClient
participant API as ThirdwebApiClient
participant HCW as ThirdwebHttpClientWrapper
participant IHC as IThirdwebHttpClient
participant Net as Network
App->>TC: Create(...)
Note right of TC: Initializes internal Api<br/>with ThirdwebHttpClientWrapper
App->>API: Call generated API methods
API->>HCW: SendAsync(HttpRequestMessage,...)
HCW->>IHC: Get/Post/Put/Delete(url, content, ct)
IHC->>Net: HTTP request
Net-->>IHC: HTTP response
IHC-->>HCW: ThirdwebHttpResponseMessage
HCW-->>API: HttpResponseMessage
API-->>App: DTO / ApiResponse
sequenceDiagram
autonumber
participant App as Caller
participant TC as ThirdwebContract
participant API as ThirdwebApiClient
participant TT as ThirdwebTransaction
App->>TC: Deploy(client, chainId, serverWalletAddress, bytecode, abi, ...)
TC->>API: DeployContractAsync(...)
API-->>TC: { contractAddress, transactionId }
TC->>TT: WaitForTransactionHash(client, transactionId)
TT-->>TC: txHash
TC->>TT: WaitForTransactionReceipt(client, txHash)
TT-->>TC: receipt
TC-->>App: deployedContractAddress
sequenceDiagram
autonumber
participant App as App Code
participant U as Utils.IsEip155Enforced
participant RPC as Local RPC
participant Net as Chain
App->>U: IsEip155Enforced(chainId,...)
U->>Net: IsDeployed(ArachnidAddr)
alt Not deployed
U->>RPC: Create local RPC and send pre-155 raw tx
RPC->>Net: Broadcast tx
Net-->>RPC: Response / Error
RPC-->>U: Result
else Deployed
Note right of U: Skip pre-155 test
end
U-->>App: bool
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ 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 (
|
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🔭 Outside diff range comments (3)
Thirdweb/Thirdweb.Wallets/SmartWallet/Thirdweb.AccountAbstraction/BundlerClient.cs (1)
113-121: Dispose IDisposable resources: CancellationTokenSource and StringContentLine [113] and Line [119] create IDisposable instances that are never disposed. Under sustained load this can leak resources. Use “using var” to ensure disposal.
Apply this diff:
- var cts = new CancellationTokenSource(client.FetchTimeoutOptions.GetTimeout(TimeoutType.Other)); + using var cts = new CancellationTokenSource(client.FetchTimeoutOptions.GetTimeout(TimeoutType.Other)); @@ - var httpContent = new StringContent(requestMessageJson, System.Text.Encoding.UTF8, "application/json"); + using var httpContent = new StringContent(requestMessageJson, System.Text.Encoding.UTF8, "application/json");If ThirdwebHttpResponseMessage implements IDisposable, consider disposing it as well (e.g.,
using var httpResponse = ...) and keep subsequent reads within the same scope.Thirdweb/Thirdweb.Indexer/ThirdwebInsight.Extensions.cs (1)
14-21: Potential NullReferenceException in exception message when Contract is null.The switch safely uses
token.Contract?.Type, but the default arm interpolatestoken.Contract.Type, which will NRE ifContractis null.Apply this diff to make the exception message null-safe:
- _ => throw new Exception($"Unknown NFT type: {token.Contract.Type}"), + _ => throw new Exception($"Unknown NFT type: {token.Contract?.Type ?? "<null>"}"),Optional: consider
NotSupportedExceptionand a case-insensitive match to avoid the duplicate "ERC721"/"erc721" arms.Thirdweb/Thirdweb.Pay/ThirdwebPay.GetBuyWithCryptoStatus.cs (1)
36-56: Prevent NullReferenceException when deserialization returns nullJsonConvert.DeserializeObject can succeed but return null. In that case, accessing error.Error.* will throw. Add a null guard and fall back to the "Unknown" error object when either error or error.Error is null.
Apply this diff within the try/catch block:
try { - error = JsonConvert.DeserializeObject<ErrorResponse>(content); + error = JsonConvert.DeserializeObject<ErrorResponse>(content); + if (error?.Error == null) + { + error = new ErrorResponse + { + Error = new ErrorDetails + { + Message = "Unknown error", + Reason = "Unknown", + Code = "Unknown", + Stack = "Unknown", + StatusCode = (int)getResponse.StatusCode, + }, + }; + } } catch { error = new ErrorResponse { Error = new ErrorDetails { Message = "Unknown error", Reason = "Unknown", Code = "Unknown", Stack = "Unknown", StatusCode = (int)getResponse.StatusCode, }, }; }
🧹 Nitpick comments (47)
Thirdweb/Thirdweb.Wallets/SmartWallet/Thirdweb.AccountAbstraction/AATypes.cs (1)
628-629: Trailing comma is fine; consider a tiny readability cleanup.No functional change here; trailing commas are valid and help cleaner diffs. As a small readability nit, you can simplify the null-check:
Apply this diff within the current lines:
- data = this.Data != null ? this.Data.BytesToHex() : "0x", + data = this.Data?.BytesToHex() ?? "0x",Thirdweb/Thirdweb.Wallets/InAppWallet/EmbeddedWallet/EmbeddedWallet.AccountLinking.cs (1)
5-17: Consider adding basic argument validation to avoid null-reference issues
linkedAccountorlinkedAccount.Detailsbeing null will throw at runtime. Light-weight guards improve resilience with negligible cost.Apply this diff:
public async Task<List<Server.LinkedAccount>> UnlinkAccountAsync(string currentAccountToken, LinkedAccount linkedAccount) { - var serverLinkedAccount = new Server.LinkedAccount + if (string.IsNullOrWhiteSpace(currentAccountToken)) + throw new ArgumentException("Current account token is required.", nameof(currentAccountToken)); + ArgumentNullException.ThrowIfNull(linkedAccount); + ArgumentNullException.ThrowIfNull(linkedAccount.Details); + + var serverLinkedAccount = new Server.LinkedAccount { Type = linkedAccount.Type, Details = new Server.LinkedAccount.LinkedAccountDetails { Email = linkedAccount.Details.Email, Address = linkedAccount.Details.Address, Phone = linkedAccount.Details.Phone, Id = linkedAccount.Details.Id, }, }; return await this._server.UnlinkAccountAsync(currentAccountToken, serverLinkedAccount).ConfigureAwait(false); }Thirdweb/Thirdweb.Wallets/SmartWallet/Thirdweb.AccountAbstraction/BundlerClient.cs (4)
115-118: Avoid assignment expression inside argument list; compute filtered args explicitlyUsing an assignment expression inside the constructor call harms readability and can be mistaken for a named argument. It also reassigns the params array unexpectedly. Compute filteredArgs separately.
Apply this diff:
- var httpClient = client.HttpClient; - var requestMessage = new RpcRequestMessage(requestId, method, args = args.Where(a => a != null).ToArray()); + var httpClient = client.HttpClient; + var filteredArgs = args?.Where(a => a != null).ToArray() ?? Array.Empty<object>(); + var requestMessage = new RpcRequestMessage(requestId, method, filteredArgs); var requestMessageJson = JsonConvert.SerializeObject(requestMessage);
131-134: Throw HttpRequestException for unsuccessful HTTP status codesThrowing a generic Exception loses semantics and complicates catch logic upstream. Use HttpRequestException instead.
Apply this diff:
- throw new Exception($"Bundler Request Failed. Error: {httpResponse.StatusCode} - {await httpResponse.Content.ReadAsStringAsync().ConfigureAwait(false)}"); + throw new HttpRequestException($"Bundler Request Failed. StatusCode: {httpResponse.StatusCode}, Body: {await httpResponse.Content.ReadAsStringAsync().ConfigureAwait(false)}");
136-140: Harden parsing: guard against null/invalid JSON-RPC response and missing resultIf deserialization fails (null) or the response carries neither error nor result, callers will hit NullReference exceptions later. Fail fast with clearer diagnostics.
Apply this diff:
- var response = JsonConvert.DeserializeObject<RpcResponseMessage>(httpResponseJson); - return response.Error != null ? throw new Exception($"Bundler Request Failed. Error: {response.Error.Code} - {response.Error.Message} - {response.Error.Data}") : response; + var response = JsonConvert.DeserializeObject<RpcResponseMessage>(httpResponseJson); + if (response == null) + throw new Exception("Bundler Request Failed. Empty or invalid JSON-RPC response."); + if (response.Error != null) + throw new Exception($"Bundler Request Failed. Error: {response.Error.Code} - {response.Error.Message} - {response.Error.Data}"); + if (response.Result == null) + throw new Exception("Bundler Request Failed. Response contained no result."); + return response;
43-47: Minor: unify requestId type across methods for consistencyThis method takes
int requestIdwhile others acceptobject requestId. Consider standardizing the type (likelyobject) to reduce surprises for callers and keep JSON-RPC ID handling consistent.Thirdweb/Thirdweb.Wallets/InAppWallet/EmbeddedWallet.Encryption/Secrets.cs (2)
154-157: Avoid magic number 1024; use the named constant to prevent driftThe validation uses a hard-coded 1024 while the exception message references Defaults.MaxPaddingMultiple. Prefer referencing the constant in the check as well to avoid discrepancies if the constant ever changes.
Apply this diff:
- else if (paddingMultiple is < 0 or > 1024) + else if (paddingMultiple is < 0 or > Defaults.MaxPaddingMultiple) { throw new ArgumentOutOfRangeException(nameof(paddingMultiple), $"{nameof(paddingMultiple)} must be in the range [0, {Defaults.MaxPaddingMultiple}]."); }
305-311: Ternary chain is correct; consider a switch expression for clarityNo functional issues. If you want to improve readability and reduce precedence surprises, a switch expression reads cleaner.
Apply this diff:
- private static int GetLargeBaseValue(char ch) - { - var rv = - ch >= 'a' ? ch - 'a' + 10 - : ch >= 'A' ? ch - 'A' + 10 - : ch - '0'; - return rv; - } + private static int GetLargeBaseValue(char ch) + { + return ch switch + { + >= 'a' and <= 'z' => ch - 'a' + 10, + >= 'A' and <= 'Z' => ch - 'A' + 10, + _ => ch - '0', + }; + }Thirdweb/Thirdweb.Wallets/InAppWallet/EmbeddedWallet.Encryption/EmbeddedWallet.Cryptography.cs (2)
16-23: Guard against malformed encryptedShare before indexing partsWe index parts[0..2] before validating the length. A quick guard improves robustness and makes errors intentional rather than IndexOutOfRangeException.
Apply this diff:
private static async Task<string> DecryptShareAsync(string encryptedShare, string password) { - var parts = encryptedShare.Split(ENCRYPTION_SEPARATOR); + var parts = encryptedShare.Split(ENCRYPTION_SEPARATOR); + if (parts.Length < 3) + { + // Malformed payload; keep error surface consistent with crypto verification failures. + throw new VerificationException(true); + } var ciphertextWithTag = Convert.FromBase64String(parts[0]); var iv = Convert.FromBase64String(parts[1]); var salt = Convert.FromBase64String(parts[2]); var iterationCount = parts.Length > 3 && int.TryParse(parts[3], out var parsedIterationCount) ? parsedIterationCount : DEPRECATED_ITERATION_COUNT; var key = await GetEncryptionKeyAsync(password, salt, iterationCount).ConfigureAwait(false);
126-126: Prefer ordinal comparison and avoid Split allocation for suffix extractionUsing StringComparison.Ordinal avoids culture sensitivity. Substring avoids an array allocation and potential overload ambiguity for Split across target frameworks.
Apply this diff:
- return !secret.StartsWith(WALLET_PRIVATE_KEY_PREFIX) ? throw new InvalidOperationException($"Corrupted share encountered {secret}") : new Account(secret.Split(WALLET_PRIVATE_KEY_PREFIX)[1]); + return secret.StartsWith(WALLET_PRIVATE_KEY_PREFIX, StringComparison.Ordinal) + ? new Account(secret.Substring(WALLET_PRIVATE_KEY_PREFIX.Length)) + : throw new InvalidOperationException($"Corrupted share encountered {secret}");Thirdweb/Thirdweb.RPC/ThirdwebRPC.cs (1)
275-281: Nit: use TotalMilliseconds (or a TimeSpan overload) to avoid subtle timing bugsUsing TimeSpan.Milliseconds returns only the millisecond component (0–999), not the total duration. If the interval is ever increased above 1 second, this would delay for an unintended shorter duration.
Consider this change:
- await ThirdwebTask.Delay(this._batchInterval.Milliseconds, this._cancellationTokenSource.Token).ConfigureAwait(false); + await ThirdwebTask.Delay((int)this._batchInterval.TotalMilliseconds, this._cancellationTokenSource.Token).ConfigureAwait(false);If ThirdwebTask.Delay has an overload accepting TimeSpan, prefer passing the TimeSpan directly instead.
Thirdweb/Thirdweb.AI/ThirdwebNebula.cs (2)
100-108: Typo: Rename local variablecontextFilertocontextFilterfor clarityMinor naming typo appears in multiple methods; renaming improves readability and avoids confusion with “filter”.
Apply the following diffs:
--- a/Thirdweb/Thirdweb.AI/ThirdwebNebula.cs +++ b/Thirdweb/Thirdweb.AI/ThirdwebNebula.cs @@ - var contextFiler = await this.PrepareContextFilter(wallet, context); + var contextFilter = await this.PrepareContextFilter(wallet, context); @@ - ContextFilter = contextFiler, + ContextFilter = contextFilter,--- a/Thirdweb/Thirdweb.AI/ThirdwebNebula.cs +++ b/Thirdweb/Thirdweb.AI/ThirdwebNebula.cs @@ - var contextFiler = await this.PrepareContextFilter(wallet, context); + var contextFilter = await this.PrepareContextFilter(wallet, context); @@ - ContextFilter = contextFiler, + ContextFilter = contextFilter,--- a/Thirdweb/Thirdweb.AI/ThirdwebNebula.cs +++ b/Thirdweb/Thirdweb.AI/ThirdwebNebula.cs @@ - var contextFiler = await this.PrepareContextFilter(wallet, context); + var contextFilter = await this.PrepareContextFilter(wallet, context); @@ - ContextFilter = contextFiler, + ContextFilter = contextFilter,--- a/Thirdweb/Thirdweb.AI/ThirdwebNebula.cs +++ b/Thirdweb/Thirdweb.AI/ThirdwebNebula.cs @@ - var contextFiler = await this.PrepareContextFilter(wallet, context); + var contextFilter = await this.PrepareContextFilter(wallet, context); @@ - ContextFilter = contextFiler, + ContextFilter = contextFilter,Also applies to: 123-132, 151-159, 185-193
251-259: Avoid awaiting WhenAll on an empty collection after filtering nullsNon-functional micro-optimization: move the empty check to after RemoveAll. This avoids awaiting WhenAll when there are no sign_transaction actions.
- if (transactionTasks == null || transactionTasks.Count == 0) - { - return null; - } - _ = transactionTasks.RemoveAll(task => task == null); + if (transactionTasks.Count == 0) + { + return null; + } + return (await Task.WhenAll(transactionTasks)).Where(tx => tx != null).ToList();Thirdweb/Thirdweb.Wallets/PrivateKeyWallet/PrivateKeyWallet.cs (2)
346-347: Prefer explicit RLP encoding call for empty access list over magic 0xc0 byteUsing a literal 0xc0 works but obscures intent. Encoding the empty access list explicitly improves readability and avoids “pre-encoded” bytes mixed into element encoding.
Apply:
- new byte[] { 0xc0 }, // AccessList, empty so short list bytes + RLP.EncodeList(Array.Empty<byte[]>()), // AccessList (empty)
408-409: Guard against double “0x” prefix in signed transaction stringLegacyTransactionSigner may already return a 0x-prefixed payload. The unconditional prefix risks returning "0x0x...". Safer to normalize the prefix once.
Proposed change:
- return Task.FromResult("0x" + signedTransaction); + var prefixed = signedTransaction.StartsWith("0x", System.StringComparison.OrdinalIgnoreCase) + ? signedTransaction + : "0x" + signedTransaction; + return Task.FromResult(prefixed);If you’d like, I can run a quick repo search to confirm current behaviors of LegacyTransactionSigner within this codebase.
Thirdweb/Thirdweb.Pay/ThirdwebPay.GetBuyHistory.cs (2)
62-62: Avoid leaking server stack traces in exception messagesIncluding
error.Error.Stackin the thrown message can leak internal server details to SDK consumers. Consider omitting it from the exception text.Apply this diff:
- throw new Exception($"HTTP error! Code: {error.Error.Code} Message: {error.Error.Message} Reason: {error.Error.Reason} StatusCode: {error.Error.StatusCode} Stack: {error.Error.Stack}"); + throw new Exception($"HTTP error! Code: {error.Error.Code} Message: {error.Error.Message} Reason: {error.Error.Reason} StatusCode: {error.Error.StatusCode}");
35-37: Consider ConfigureAwait(false) for library codeTo keep library awaits context-agnostic (and consistent with other files in this repo), consider adding ConfigureAwait(false) to HttpClient awaits.
Example:
var getResponse = await client.HttpClient.GetAsync(url).ConfigureAwait(false); var content = await getResponse.Content.ReadAsStringAsync().ConfigureAwait(false);Also applies to: 62-62
Thirdweb/Thirdweb.Pay/ThirdwebPay.GetBuyWithFiatCurrencies.cs (2)
41-42: Don’t propagate server stack traces in error stringsSame rationale as other Pay endpoints: avoid including
error.Error.Stackin thrown messages to prevent leaking server internals.Apply this diff:
- throw new Exception($"HTTP error! Code: {error.Error.Code} Message: {error.Error.Message} Reason: {error.Error.Reason} StatusCode: {error.Error.StatusCode} Stack: {error.Error.Stack}"); + throw new Exception($"HTTP error! Code: {error.Error.Code} Message: {error.Error.Message} Reason: {error.Error.Reason} StatusCode: {error.Error.StatusCode}");Also applies to: 46-46
20-23: Align awaits with library best practicesConsider
.ConfigureAwait(false)on awaits to avoid capturing context in library code.Example:
var getResponse = await client.HttpClient.GetAsync(url).ConfigureAwait(false); var content = await getResponse.Content.ReadAsStringAsync().ConfigureAwait(false);Also applies to: 46-46
Thirdweb/Thirdweb.Utils/Utils.cs (3)
1148-1161: Defensive decode for contract-creation transactions (null “to”)For contract creation,
tocan be null/empty. CallingToChecksumAddress()unconditionally risks a null reference. Use a null-conditional and default data to "0x" for safety.Apply this diff:
return ( new ThirdwebTransactionInput( chainId: chainId, - to: receiverAddress.ToChecksumAddress(), + to: receiverAddress?.ToChecksumAddress(), nonce: nonce, gas: gasLimit, value: amount, - data: data, + data: data ?? "0x", maxFeePerGas: maxFeePerGas, maxPriorityFeePerGas: maxPriorityFeePerGas ) { AuthorizationList = authorizations, }, signature.CreateStringSignature() );
1168-1195: Typo: DecodeAutorizationList -> DecodeAuthorizationList (public API)The method name is misspelled. Renaming would be breaking since it’s public. Consider adding a correctly spelled alias method and marking the current one
[Obsolete]to preserve backwards compatibility.Would you like me to draft the alias method and update call sites in this PR or a follow-up?
1225-1225: Confirm analytics schema for gas price numeric size
gasPricemay exceed 64-bit integer limits. Json.NET will serializeBigIntegeras a JSON number, which can overflow downstream systems expecting IEEE-754 doubles. If the ingestion pipeline expects strings for large integers, serialize as string.Option if strings are acceptable downstream:
- gasPrice = transaction.Input.GasPrice?.Value ?? transaction.Input.MaxFeePerGas?.Value, + gasPrice = (transaction.Input.GasPrice?.Value ?? transaction.Input.MaxFeePerGas?.Value)?.ToString(),If the pipeline expects a number, keep as-is.
Please confirm the expected type with the analytics backend before changing.
Thirdweb/Thirdweb.Wallets/InAppWallet/EmbeddedWallet.Authentication/Server.cs (4)
111-116: Minor readability nit: simplify boolean-to-string conversion.Not required, but this reads a bit cleaner and avoids manual ternaries.
- { - { "getEncryptedAuthShare", "true" }, - { "getEncryptedRecoveryShare", wantsRecoveryShare ? "true" : "false" }, - { "useSealedSecret", "false" }, - }; + { + { "getEncryptedAuthShare", true.ToString().ToLowerInvariant() }, + { "getEncryptedRecoveryShare", wantsRecoveryShare.ToString().ToLowerInvariant() }, + { "useSealedSecret", false.ToString().ToLowerInvariant() }, + };
391-398: Harden query-string construction (escape keys, skip null values).Current code escapes only values and will throw if any value is null. This version also escapes keys and filters out nulls.
- if (parameters != null && parameters.Any()) - { - var queryString = string.Join('&', parameters.Select((p) => $"{p.Key}={Uri.EscapeDataString(p.Value)}")); - b.Query = queryString; - } + if (parameters != null && parameters.Any()) + { + var queryString = string.Join("&", + parameters + .Where(p => p.Value != null) + .Select(p => $"{Uri.EscapeDataString(p.Key)}={Uri.EscapeDataString(p.Value)}")); + b.Query = queryString; + }Same suggestion applies to MakeUri2023 below.
402-409: Mirror the safer query logic from MakeUri2024.See the comment above; apply the same key-escaping and null filtering here.
- if (parameters != null && parameters.Any()) - { - var queryString = string.Join('&', parameters.Select((p) => $"{p.Key}={Uri.EscapeDataString(p.Value)}")); - b.Query = queryString; - } + if (parameters != null && parameters.Any()) + { + var queryString = string.Join("&", + parameters + .Where(p => p.Value != null) + .Select(p => $"{Uri.EscapeDataString(p.Key)}={Uri.EscapeDataString(p.Value)}")); + b.Query = queryString; + }
335-364: Avoid shared mutable header state on _httpClient to prevent race conditions.
AddHeader("Authorization", ...)+RemoveHeader("Authorization")on a shared client can interleave across concurrent calls, causing requests to be sent with wrong or missing headers.Options:
- Prefer per-request headers: extend
IThirdwebHttpClientto accept a headers dictionary for each call, or add overloads likePostAsync(string url, HttpContent content, IDictionary<string,string> headers).- Alternatively, use a dedicated, short-lived client/handler for authenticated calls, or a DelegatingHandler that injects headers based on an ambient context.
- If the wrapper must keep this API, ensure
AddHeader/RemoveHeaderare thread-safe and scoped (e.g., use an async-local header bag) so concurrent calls don't interfere.Would you like a small refactor proposal for
IThirdwebHttpClientto support per-request headers?Thirdweb/Thirdweb.csproj (1)
4-6: Consider stabilizing AssemblyVersion to Major.Minor.0.0 to avoid binding friction.Keeping AssemblyVersion fixed across patch releases reduces downstream binding issues; FileVersion/PackageVersion can still track the exact release.
- <AssemblyVersion>2.24.1</AssemblyVersion> + <AssemblyVersion>2.24.0.0</AssemblyVersion>If you prefer aligning all three, feel free to ignore; this is a common library practice recommendation.
Thirdweb/Thirdweb.Wallets/InAppWallet/EcosystemWallet/EcosystemWallet.cs (1)
841-844: Avoid treating empty email string as a valid email when verifying OTPThe current ternary uses
this.Email == nullto choose Phone vs Email verification, but the guard above usesstring.IsNullOrEmpty(this.Email)when determining required inputs. This mismatch can route an empty string down the Email path and lead to avoidable failures.Consider consistently using
string.IsNullOrEmpty(this.Email)here.Apply:
- 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); + var serverRes = + string.IsNullOrEmpty(this.Email) && string.IsNullOrEmpty(this.PhoneNumber) + ? throw new Exception("Email or Phone Number is required for OTP login") + : string.IsNullOrEmpty(this.Email) + ? await this.EmbeddedWallet.VerifyPhoneOtpAsync(this.PhoneNumber, otp).ConfigureAwait(false) + : await this.EmbeddedWallet.VerifyEmailOtpAsync(this.Email, otp).ConfigureAwait(false);Thirdweb/Thirdweb.Wallets/SmartWallet/SmartWallet.cs (1)
56-99: Hard-coded ERC20 paymaster config: consider making this data-driven and verifiableThe static dictionary is clear and guarded by chainId checks. That said:
- Token paymasters and proxies can migrate; addresses/balance slots may change.
- Surfacing an extension point (appsettings/env or a constructor override) would let advanced users supply/override configs without changing code.
- Consider validating addresses (checksum) at startup and adding a short doc comment explaining BalanceStorageSlot selection.
If you want, I can sketch a simple opt-in override (IReadOnlyDictionary parameter) while preserving current defaults.
.editorconfig (2)
36-41: CRLF enforcement: consider portabilityForcing CRLF for all C# files can be noisy on non-Windows environments. If you have cross-platform contributors/CI, consider omitting end_of_line or using
lfwith a .gitattributes policy instead.
187-188: Silencing JSON002 globallyIf the warning is primarily noisy in generated code, you could scope this suppression under the generated files section. Keeping it global is OK if you regularly embed JSON in strings in hand-written code.
Thirdweb/Thirdweb.Bridge/ThirdwebBridge.Types.cs (1)
396-404: Map PROCESSING and CREATED to avoid UNKNOWN in StatusDataStatusData.StatusType currently treats PROCESSING/CREATED as UNKNOWN, unlike OnrampStatusData which maps them explicitly. If the backend can return these values here, callers lose fidelity.
Apply this diff to add the missing mappings:
public StatusType StatusType => 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, };Thirdweb/Thirdweb.Bridge/ThirdwebBridge.cs (1)
513-526: URL-encode query parameters and handle existing query stringsAppendQueryParams concatenates raw keys/values without encoding and assumes no existing query in the base URL. While current values are mostly safe (hex/numeric), encoding is safer and future-proof.
Apply this diff:
- private static string AppendQueryParams(string url, Dictionary<string, string> queryParams) - { - var query = new List<string>(); - foreach (var param in queryParams) - { - if (string.IsNullOrEmpty(param.Value)) - { - continue; - } - query.Add($"{param.Key}={param.Value}"); - } - - return url + "?" + string.Join("&", query); - } + private static string AppendQueryParams(string url, Dictionary<string, string> queryParams) + { + var parts = new List<string>(); + foreach (var param in queryParams) + { + if (string.IsNullOrEmpty(param.Value)) + { + continue; + } + var key = Uri.EscapeDataString(param.Key); + var val = Uri.EscapeDataString(param.Value); + parts.Add($"{key}={val}"); + } + if (parts.Count == 0) + { + return url; + } + var separator = url.Contains("?") ? "&" : "?"; + return url + separator + string.Join("&", parts); + }nswag.json (2)
37-37: Optional: Revisit generateOptionalParametersWith generateOptionalParameters = false, optional query params become mandatory in method signatures, which can make call sites noisier.
If the API defines many optional params, setting this to true can make the generated client more ergonomic.
43-46: Mismatch: generateResponseClasses without wrapping responsesCurrently wrapResponses = false but generateResponseClasses = true with responseClass = ApiResponse. This likely emits the ApiResponse type but doesn’t use it.
Either:
- Enable wrapResponses if you want access to headers/status codes; or
- Set generateResponseClasses to false to avoid generating unused types.
tw.bat (3)
1-4: Run from script directory and scope environment changesUsing repo-relative paths assumes the current working directory. Ensure the script runs from its own directory and cleanly restores the previous directory.
Apply this diff:
@echo off +setlocal +pushd "%~dp0" REM Thirdweb Build Script REM Usage: tw.bat [command] @@ -:end +:end +popd +endlocalAlso applies to: 146-147
99-106: CSharpier command name may be dotnet-csharpier on WindowsGlobal tool installs often expose dotnet-csharpier.exe (not csharpier.exe). Your check may fail even if installed.
Option A (simple): Replace csharpier with dotnet-csharpier in both lint/fix blocks.
Option B (robust): Try csharpier, then fallback to dotnet-csharpier. I can provide a patch if you want the fallback behavior.
Also applies to: 114-121
34-43: Optional: NSwag path robustnessAfter dotnet tool install, PATH may not refresh within the same session. Consider invoking %USERPROFILE%.dotnet\tools\nswag.exe as a fallback if nswag isn’t found on PATH.
I can provide a small patch to set NSWAG_EXE to either nswag or the absolute tools path and use it consistently.
Also applies to: 47-51
Thirdweb/Thirdweb.Pay/ThirdwebPay.GetBuyWithCryptoQuote.cs (1)
41-49: Consider surfacing raw response content in fallback error pathCurrent fallback sets generic placeholders when deserialization fails. Including a snippet of raw content greatly improves diagnosability without changing control flow.
Apply this diff within the fallback initializer to include raw content safely:
- Error = new ErrorDetails - { - Message = "Unknown error", - Reason = "Unknown", - Code = "Unknown", - Stack = "Unknown", - StatusCode = (int)response.StatusCode, - }, + Error = new ErrorDetails + { + Message = string.IsNullOrWhiteSpace(content) ? "Unknown error" : $"Unknown error. Raw: {content}", + Reason = "Unknown", + Code = "Unknown", + Stack = "Unknown", + StatusCode = (int)response.StatusCode, + },Thirdweb/Thirdweb.Pay/ThirdwebPay.GetBuyWithFiatQuote.cs (1)
41-49: Mirror the crypto path: include raw content on deserialization failureSame note as in GetBuyWithCryptoQuote: surfacing the raw response when JSON parsing fails helps debugging API issues.
Apply this diff in the fallback initializer:
- Error = new ErrorDetails - { - Message = "Unknown error", - Reason = "Unknown", - Code = "Unknown", - Stack = "Unknown", - StatusCode = (int)response.StatusCode, - }, + Error = new ErrorDetails + { + Message = string.IsNullOrWhiteSpace(content) ? "Unknown error" : $"Unknown error. Raw: {content}", + Reason = "Unknown", + Code = "Unknown", + Stack = "Unknown", + StatusCode = (int)response.StatusCode, + },Thirdweb/Thirdweb.Pay/ThirdwebPay.GetBuyWithFiatStatus.cs (1)
36-56: Harden error handling against null JSON payloadsIf deserialization returns null (valid JSON "null"), the subsequent interpolation will dereference null. Add a final null-coalescing fallback before throwing.
Apply this diff:
if (!getResponse.IsSuccessStatusCode) { ErrorResponse error; try { error = JsonConvert.DeserializeObject<ErrorResponse>(content); } catch { error = new ErrorResponse { Error = new ErrorDetails { Message = "Unknown error", Reason = "Unknown", Code = "Unknown", Stack = "Unknown", StatusCode = (int)getResponse.StatusCode, }, }; } - throw new Exception($"HTTP error! Code: {error.Error.Code} Message: {error.Error.Message} Reason: {error.Error.Reason} StatusCode: {error.Error.StatusCode} Stack: {error.Error.Stack}"); + // Ensure a non-null error object even if deserialization yielded null + if (error == null || error.Error == null) + { + error = new ErrorResponse + { + Error = new ErrorDetails + { + Message = "Unknown error", + Reason = "Unknown", + Code = "Unknown", + Stack = "Unknown", + StatusCode = (int)getResponse.StatusCode, + }, + }; + } + throw new Exception($"HTTP error! Code: {error.Error.Code} Message: {error.Error.Message} Reason: {error.Error.Reason} StatusCode: {error.Error.StatusCode} Stack: {error.Error.Stack}"); }Thirdweb/Thirdweb.Client/ThirdwebClient.cs (3)
23-28: Fix doc typo and tighten descriptionMinor nit: “Interactiton” → “Interaction”.
- /// Interactiton with https://api.thirdweb.com - /// Used in some places to enhance the core SDK functionality, or even extend it + /// Interaction with https://api.thirdweb.com. + /// Used to enhance or extend core SDK functionality where needed.
81-91: Avoid type/property ambiguity and prepare for consistent timeouts/disposal
- There’s a class property named HttpClient; fully qualify System.Net.Http.HttpClient here to avoid any ambiguity and improve readability.
- Consider aligning apiHttpClient.Timeout with your FetchTimeoutOptions and ensuring proper disposal (see next comment).
Apply this diff:
- var apiHttpClient = new HttpClient { BaseAddress = new Uri("https://api.thirdweb.com") }; + var apiHttpClient = new System.Net.Http.HttpClient { BaseAddress = new Uri("https://api.thirdweb.com") };Additionally consider (outside this diff) setting a timeout consistent with FetchTimeoutOptions to avoid indefinite hangs when the API is slow.
81-91: Consider disposing the API HttpClient (or making the client disposable)The new System.Net.Http.HttpClient instance is never disposed. If multiple ThirdwebClient instances can be created, this risks socket exhaustion. Two options:
- Make ThirdwebClient implement IDisposable and dispose the internal apiHttpClient (and delegate disposal of IThirdwebHttpClient if it’s disposable).
- Or share a static, long-lived HttpClient for the API.
Example (outside current ranges):
public class ThirdwebClient : IDisposable { private readonly System.Net.Http.HttpClient _apiHttpClient; // keep a field // in ctor: _apiHttpClient = new System.Net.Http.HttpClient { BaseAddress = new Uri("https://api.thirdweb.com") }; this.Api = new ThirdwebApiClient(_apiHttpClient); public void Dispose() { _apiHttpClient?.Dispose(); (this.HttpClient as IDisposable)?.Dispose(); } }Thirdweb/Thirdweb.Extensions/ThirdwebExtensions.cs (2)
110-113: Ternary chain reduces readability; consider early returnsThe condensed ternary is correct but less readable for a hot utility method. An explicit early-return version tends to be clearer and friendlier to debuggers.
- 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 (string.IsNullOrEmpty(nft.Metadata.Image)) return Array.Empty<byte>(); + return await ThirdwebStorage.Download<byte[]>(client, nft.Metadata.Image).ConfigureAwait(false);
1783-1785: Likely copy/paste: call ERC721 variant instead of ERC20This ERC721 helper currently calls DropERC20_GetClaimConditionById. It probably should call DropERC721_GetClaimConditionById for clarity and consistency (even if the underlying ABI is the same).
- return await contract.DropERC20_GetClaimConditionById(activeClaimConditionId); + return await contract.DropERC721_GetClaimConditionById(activeClaimConditionId);Thirdweb.Console/Program.cs (1)
15-36: Helpful deploy sample; consider noting required secrets and cancellationThe commented deploy sample is clear. Consider adding a short comment that:
- THIRDWEB_SECRET_KEY must be set server-side (never in client apps).
- Deploy supports cancellation via the provided token to avoid indefinite waits.
Example comment to add above the region:
+# Note: Requires THIRDWEB_SECRET_KEY (server-side only). Do not use secrets client-side. +# You can pass a CancellationToken to ThirdwebContract.Deploy to bound the wait time.
Introduced ThirdwebHttpClientWrapper to adapt IThirdwebHttpClient for use with the generated API client, replacing direct usage of System.Net.Http.HttpClient. Updated the API client, client initialization, and NSwag config to use the wrapper. Also improved API documentation, added support for ERC20 token balance queries, and made minor fixes to comments and method signatures.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (12)
Thirdweb.Console/Program.cs (5)
15-35: Consider gating the deploy sample to avoid accidental on-chain writesThis region will always execute a contract deployment when the console app runs (even if it’s “just” Sepolia). Recommend gating with an env var or a simple runtime confirmation to prevent unintended spends during local runs/CI.
Example approach:
- Guard behind an env var, e.g., RUN_DEPLOY_SAMPLE=1
- Or prompt the user before proceeding in interactive runs
Do you want me to wrap this region behind an env var and push a follow-up change?
17-18: Parameterize the server wallet label and add failure loggingHardcoding the label risks brittle demos. Reading it from an env var and surfacing a clear error improves DX.
Apply this diff:
-var serverWallet = await ServerWallet.Create(client: client, label: "TestFromDotnet"); +var walletLabel = Environment.GetEnvironmentVariable("TW_SERVER_WALLET_LABEL") ?? "TestFromDotnet"; +ServerWallet serverWallet; +try +{ + serverWallet = await ServerWallet.Create(client: client, label: walletLabel); +} +catch (Exception ex) +{ + Console.Error.WriteLine($"Failed to create ServerWallet with label '{walletLabel}': {ex.Message}"); + throw; +}
19-21: Prefer a raw string literal for the ABI for readability (if on C# 11+)The escaped JSON is harder to read/maintain. Raw string keeps it clean. If you’re not on C# 11 yet, ignore this.
Apply this diff (C# 11+):
-var abi = - "[ { \"inputs\": [], \"name\": \"welcome\", \"outputs\": [ { \"internalType\": \"string\", \"name\": \"\", \"type\": \"string\" } ], \"stateMutability\": \"pure\", \"type\": \"function\" } ]"; +var abi = """ +[ + { + "inputs": [], + "name": "welcome", + "outputs": [ + { + "internalType": "string", + "name": "", + "type": "string" + } + ], + "stateMutability": "pure", + "type": "function" + } +] +""";If you’d like, I can verify the project’s LangVersion/TargetFramework in csproj files and suggest the right variant.
22-28: Add timeout/cancellation and remove inline awaits for clarityA bounded cancellation window prevents indefinite waits on flaky networks. Also factoring chainId and address improves readability and loggability.
Apply this diff:
-var contractAddress = await ThirdwebContract.Deploy( - client: client, - chainId: 11155111, - serverWalletAddress: await serverWallet.GetAddress(), - bytecode: "6080604052348015600e575f5ffd5b5061014e8061001c5f395ff3fe608060405234801561000f575f5ffd5b5060043610610029575f3560e01c8063b627cf3b1461002d575b5f5ffd5b61003561004b565b60405161004291906100f8565b60405180910390f35b60606040518060400160405280601481526020017f57656c636f6d6520746f20746869726477656221000000000000000000000000815250905090565b5f81519050919050565b5f82825260208201905092915050565b8281835e5f83830152505050565b5f601f19601f8301169050919050565b5f6100ca82610088565b6100d48185610092565b93506100e48185602086016100a2565b6100ed816100b0565b840191505092915050565b5f6020820190508181035f83015261011081846100c0565b90509291505056fea264697066735822122001498e9d7d6125ce22613ef32fdb7e8e03bf11ad361d7b00e210b82d7b7e0d4464736f6c634300081e0033", - abi: abi -); +var chainId = 11155111; // Sepolia +var serverWalletAddress = await serverWallet.GetAddress(); +using var cts = new System.Threading.CancellationTokenSource(TimeSpan.FromMinutes(2)); +var contractAddress = await ThirdwebContract.Deploy( + client: client, + chainId: chainId, + serverWalletAddress: serverWalletAddress, + bytecode: "6080604052348015600e575f5ffd5b5061014e8061001c5f395ff3fe608060405234801561000f575f5ffd5b5060043610610029575f3560e01c8063b627cf3b1461002d575b5f5ffd5b61003561004b565b60405161004291906100f8565b60405180910390f35b60606040518060400160405280601481526020017f57656c636f6d6520746f20746869726477656221000000000000000000000000815250905090565b5f81519050919050565b5f82825260208201905092915050565b8281835e5f83830152505050565b5f601f19601f8301169050919050565b5f6100ca82610088565b6100d48185610092565b93506100e48185602086016100a2565b6100ed816100b0565b840191505092915050565b5f6020820190508181035f83015261011081846100c0565b90509291505056fea264697066735822122001498e9d7d6125ce22613ef32fdb7e8e03bf11ad361d7b00e210b82d7b7e0d4464736f6c634300081e0033", + abi: abi, + cancellationToken: cts.Token +);Note: Using a salt (Deploy’s optional parameter) can make deployments deterministic (CREATE2). If that’s desirable for your flow, we can wire it to an env var (e.g., TW_DEPLOY_SALT).
31-33: Disambiguate the function call with a canonical signatureMinor robustness: passing “welcome()” avoids any future ambiguity if the ABI gains an overloaded function.
Apply this diff:
-var welcomeMessage = await contract.Read<string>("welcome"); +var welcomeMessage = await contract.Read<string>("welcome()");.editorconfig (1)
36-40: Cross-platform line endings: double-check CRLF requirement.
end_of_line = crlfglobally enforces Windows line endings. If contributors build on macOS/Linux, this may cause churn. If not required, consider leavingend_of_lineunset (default platform-native) or manage via.gitattributesinstead.Thirdweb/Thirdweb.Http/ThirdwebHttpClient.cs (1)
107-121: Consider implementing PUT and DELETE to avoid runtime NotImplementedException.Your wrapper exposes
PutAsync/DeleteAsyncthrough the interface and the NSwag client can call these operations in the future. Implementing them now avoids surprises.Example implementation:
public Task<ThirdwebHttpResponseMessage> PutAsync(string requestUri, HttpContent content, CancellationToken cancellationToken = default) { - throw new NotImplementedException(); + var request = new HttpRequestMessage(HttpMethod.Put, requestUri) { Content = content }; + this.AddHeaders(request); + return SendAsyncInternal(request, cancellationToken); } @@ public Task<ThirdwebHttpResponseMessage> DeleteAsync(string requestUri, CancellationToken cancellationToken = default) { - throw new NotImplementedException(); + var request = new HttpRequestMessage(HttpMethod.Delete, requestUri); + this.AddHeaders(request); + return SendAsyncInternal(request, cancellationToken); }Where
SendAsyncInternalcan be a small helper duplicating the GET/POST content handling, to DRY the code.Thirdweb/Thirdweb.Api/ThirdwebHttpClientWrapper.cs (3)
20-21: Prefer overriding the protected virtual SendAsync instead of hiding the public overload.Given NSwag
httpClientType = "ThirdwebHttpClientWrapper", yournewmethod works today. Overriding the protected virtual is more robust if this type is ever used through HttpClient or if generator settings change.Apply this diff:
- public new async Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, HttpCompletionOption completionOption, CancellationToken cancellationToken) + protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, HttpCompletionOption completionOption, CancellationToken cancellationToken)The existing
public override Task<HttpResponseMessage> SendAsync(HttpRequestMessage, CancellationToken)can remain and delegate to the completion-option overload.
24-41: Per-request headers are not propagated to IThirdwebHttpClient.NSwag-generated requests often set headers (e.g., Accept, Authorization) on
HttpRequestMessage. These are not forwarded; the underlying client only uses its own global header dictionary. If endpoints rely on Accept/Auth headers being present, requests may be malformed.Options:
- Short-term: copy request headers into
_thirdwebClient.Headersbefore the call and restore afterwards (beware of concurrency).- Better: extend
IThirdwebHttpClientmethods to accept per-request headers.If you want the minimal approach now, I can provide a thread-safe copying strategy using an immutable snapshot for each call.
46-49: Preserve raw bytes when creating HttpResponseMessage content.You convert content to string, which can corrupt binary or non-UTF8 payloads. Prefer
ByteArrayContent. Also setresponse.RequestMessage = requestto preserve context.Example:
- var response = new HttpResponseMessage((System.Net.HttpStatusCode)thirdwebResponse.StatusCode) { Content = new StringContent(await thirdwebResponse.Content.ReadAsStringAsync()) }; + var bytes = await thirdwebResponse.Content.ReadAsByteArrayAsync(); + var response = new HttpResponseMessage((System.Net.HttpStatusCode)thirdwebResponse.StatusCode) + { + Content = new ByteArrayContent(bytes), + RequestMessage = request + };If you can surface content-type from
ThirdwebHttpResponseMessage, also map it toresponse.Content.Headers.ContentType.nswag.json (2)
6-9: Pin the OpenAPI document to a specific version to avoid drift.Referencing
https://api.thirdweb.com/openapi.jsonwithout a version can lead to unintended breaking changes upon regeneration. Prefer a versioned URL or check in a vetted spec file.Example approaches:
- Use a versioned URL or tag parameter if supported by the API.
- Vendor the OpenAPI spec into the repo and reference it via
fromDocument.file.
12-19: Fully-qualify httpClientType to reduce ambiguity.While your wrapper shares the same namespace as the generated client, explicitly qualifying avoids subtle issues.
- "httpClientType": "ThirdwebHttpClientWrapper", + "httpClientType": "Thirdweb.Api.ThirdwebHttpClientWrapper",This also makes the NSwag config more resilient to namespace moves.
📜 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 (7)
.editorconfig(2 hunks)Thirdweb.Console/Program.cs(1 hunks)Thirdweb/Thirdweb.Api/ThirdwebHttpClientWrapper.cs(1 hunks)Thirdweb/Thirdweb.Client/ThirdwebClient.cs(3 hunks)Thirdweb/Thirdweb.Http/IThirdwebHttpClient.cs(3 hunks)Thirdweb/Thirdweb.Http/ThirdwebHttpClient.cs(2 hunks)nswag.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- Thirdweb/Thirdweb.Client/ThirdwebClient.cs
🧰 Additional context used
🧬 Code Graph Analysis (4)
Thirdweb/Thirdweb.Http/ThirdwebHttpClient.cs (1)
Thirdweb/Thirdweb.Http/ThirdwebHttpContent.cs (4)
ThirdwebHttpContent(8-81)ThirdwebHttpContent(17-25)ThirdwebHttpContent(32-35)ThirdwebHttpContent(42-52)
Thirdweb/Thirdweb.Http/IThirdwebHttpClient.cs (3)
Thirdweb/Thirdweb.Http/ThirdwebHttpClient.cs (8)
SetHeaders(29-32)ClearHeaders(37-40)AddHeader(47-50)RemoveHeader(56-59)Task(75-82)Task(91-98)Task(107-110)Task(118-121)Thirdweb/Thirdweb.RPC/ThirdwebRPC.cs (3)
Task(87-157)Task(168-255)Task(273-301)Thirdweb/Thirdweb.Http/ThirdwebHttpResponseMessage.cs (2)
ThirdwebHttpResponseMessage(12-43)ThirdwebHttpResponseMessage(34-42)
Thirdweb/Thirdweb.Api/ThirdwebHttpClientWrapper.cs (3)
Thirdweb/Thirdweb.Utils/Utils.cs (1)
IThirdwebHttpClient(979-987)Thirdweb/Thirdweb.Http/IThirdwebHttpClient.cs (4)
Task(43-43)Task(52-52)Task(61-61)Task(69-69)Thirdweb/Thirdweb.Http/ThirdwebHttpClient.cs (4)
Task(75-82)Task(91-98)Task(107-110)Task(118-121)
Thirdweb.Console/Program.cs (2)
Thirdweb/Thirdweb.Wallets/ServerWallet/ServerWallet.cs (2)
ServerWallet(14-455)ServerWallet(26-32)Thirdweb/Thirdweb.Contracts/ThirdwebContract.cs (2)
ThirdwebContract(13-318)ThirdwebContract(23-29)
🪛 GitHub Check: build-test-cov
Thirdweb/Thirdweb.Http/ThirdwebHttpClient.cs
[failure] 80-80:
No overload for method 'ReadAsByteArrayAsync' takes 1 arguments
[failure] 80-80:
No overload for method 'ReadAsByteArrayAsync' takes 1 arguments
[failure] 96-96:
No overload for method 'ReadAsByteArrayAsync' takes 1 arguments
[failure] 96-96:
No overload for method 'ReadAsByteArrayAsync' takes 1 arguments
🪛 GitHub Actions: Build and Test
Thirdweb/Thirdweb.Http/ThirdwebHttpClient.cs
[error] 80-80: dotnet build failed: CS1501: No overload for method 'ReadAsByteArrayAsync' takes 1 arguments.
🔇 Additional comments (2)
Thirdweb.Console/Program.cs (1)
39-41: Good call on de-emphasizing private-key wallets in the sampleMoving the runnable path to ServerWallet aligns with safer server-side practices and reduces the risk of key leakage in sample code.
Thirdweb/Thirdweb.Http/IThirdwebHttpClient.cs (1)
11-69: Surface area alignment looks good.Making the members explicitly public is fine (albeit redundant in interfaces). The cancellation token defaults on the interface match the implementation and help propagate cancellation through the wrapper.
Added a check to verify if the Arachnid contract is deployed before attempting to send a pre-EIP-155 raw transaction. This prevents unnecessary transaction attempts and error handling when the contract is not present.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
Thirdweb/Thirdweb.Utils/Utils.cs (3)
690-734: EIP-155 probe short-circuit is a good optimization; consider constant extraction, culture-invariant error parsing, and potential false negatives
- Good: Avoiding the pre-155 probe when the Arachnid deployer exists will reduce noisy failing tx submissions.
- Maintainability: Avoid the hard-coded Arachnid address; extract to a Constants field to keep a single source of truth.
- Robustness: Error parsing should be culture-invariant and null-safe to avoid edge cases in non-English locales.
Suggested change (culture-invariant, null-safe error message handling):
- var errorMsg = e.Message.ToLower(); + var errorMsg = e.Message?.ToLowerInvariant() ?? string.Empty;
- Heuristic caveat: Presence of the Arachnid deployer implies the chain accepted pre-155 tx at least once, but does not absolutely guarantee the current rule set on long-lived networks that may have forked since. If misclassification is risky for your flows, consider one of:
- Add an optional forceProbe flag to still run the pre-155 test even when the deployer exists (opt-in from callers when certainty is required).
- TTL-based cache with periodic re-probe for chains where correctness is critical.
- Concurrency (optional): The static Dictionary caches are not thread-safe for concurrent writers. A ConcurrentDictionary with GetOrAdd would be safer.
Optional refactor for the cache (outside of this hunk):
// field // using System.Collections.Concurrent; private static readonly ConcurrentDictionary<BigInteger, bool> _eip155EnforcedCache = new(); // method shape public static async Task<bool> IsEip155Enforced(ThirdwebClient client, BigInteger chainId) { return await _eip155EnforcedCache.GetOrAdd(chainId, _ => Task.Run(async () => { var result = false; var isArachnidDeployed = await IsDeployed(client, chainId, /* Constants.ARACHNID_CREATE2_DEPLOYER */ "0x4e59b44847b379578588920ca78fbf26c0b4956c").ConfigureAwait(false); if (!isArachnidDeployed) { try { var rpc = ThirdwebRPC.GetRpcInstance(client, chainId); var rawTransaction = "0xf8a58085174876e800830186a08080b853604580600e600039806000f350fe7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffe03601600081602082378035828234f58015156039578182fd5b8082525050506014600cf31ba02222222222222222222222222222222222222222222222222222222222222222a02222222222222222222222222222222222222222222222222222222222222222"; _ = await rpc.SendRequestAsync<string>("eth_sendRawTransaction", rawTransaction).ConfigureAwait(false); } catch (Exception e) { var errorMsg = e.Message?.ToLowerInvariant() ?? string.Empty; var errorSubstrings = new List<string> { "eip-155", "eip155", "protected", "invalid chain id for signer", "chain id none", "chain_id mismatch", "recovered sender mismatch", "transaction hash mismatch", "chainid no support", "chainid (0)", "chainid(0)", "invalid sender", }; if (errorSubstrings.Any(errorMsg.Contains)) { result = true; } else { result = _errorSubstringsComposite.Any(arr => arr.All(errorMsg.Contains)); } } } return result; })); }
967-975: Guard against null baseFeePerGas and nodes lacking eth_maxPriorityFeePerGasSome nodes (legacy or misconfigured) may omit baseFeePerGas, causing a NullReference at baseBlockFee.Value. Suggest a defensive fallback to gasPrice when baseFeePerGas is missing.
- var baseBlockFee = block["baseFeePerGas"]?.ToObject<HexBigInteger>(); - var maxFeePerGas = baseBlockFee.Value * 2; - var maxPriorityFeePerGas = ((await rpc.SendRequestAsync<HexBigInteger>("eth_maxPriorityFeePerGas").ConfigureAwait(false))?.Value) ?? (maxFeePerGas / 2); + var baseBlockFee = block["baseFeePerGas"]?.ToObject<HexBigInteger>(); + if (baseBlockFee == null) + { + // Node does not report baseFeePerGas (non-EIP-1559 or legacy RPC); fall back to gasPrice. + var gasPrice = await FetchGasPrice(client, chainId, withBump).ConfigureAwait(false); + return (gasPrice, gasPrice); + } + var maxFeePerGas = baseBlockFee.Value * 2; + var priority = await rpc.SendRequestAsync<HexBigInteger>("eth_maxPriorityFeePerGas").ConfigureAwait(false); + var maxPriorityFeePerGas = priority?.Value ?? (maxFeePerGas / 2);
1194-1194: Trailing comma OK; consider correcting long-standing method name typo (nit)The comma change is fine. Minor nit: the method is named DecodeAutorizationList (missing the ‘h’). If you plan to address, add a correctly spelled overload and mark the old one [Obsolete] to preserve binary compatibility. I can prep that refactor if useful.
📜 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 (1)
Thirdweb/Thirdweb.Utils/Utils.cs(5 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). (1)
- GitHub Check: build-test-cov
🔇 Additional comments (2)
Thirdweb/Thirdweb.Utils/Utils.cs (2)
1163-1164: Trailing comma in object initializer — fineThe stylistic change is harmless and matches common formatting preferences.
1229-1229: Telemetry payload gas field — LGTMSelecting GasPrice or MaxFeePerGas is sensible for mixed 1559/legacy chains. No issues.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #155 +/- ##
===========================================
- Coverage 58.21% 38.91% -19.31%
===========================================
Files 43 45 +2
Lines 6873 10236 +3363
Branches 875 1548 +673
===========================================
- Hits 4001 3983 -18
- Misses 2686 6063 +3377
- Partials 186 190 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Added comprehensive input validation and detailed exception handling to ThirdwebContract.Deploy and ThirdwebTransaction.WaitForTransactionHash methods. Updated .editorconfig to refine generated file exclusion. Reordered and uncommented wallet interaction and ZkSync AA code in Program.cs for clarity. Improved error handling and exit codes in tw.bat for better script reliability.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
.editorconfig (1)
11-31: Future-proof the exclusion: prefer a dedicated Generated/ folder and a narrow glob.If you anticipate additional generated files, move them under Thirdweb/Thirdweb.Api/Generated/ and narrow the scope to that directory. This avoids accidental suppression and keeps the rule resilient to file renames.
Apply this diff once you relocate the generated output:
-# Exclude Thirdweb.Api generated files from all linting and formatting rules -[Thirdweb/Thirdweb.Api/GeneratedClient.cs] +# Exclude Thirdweb.Api generated files from all linting and formatting rules +[Thirdweb/Thirdweb.Api/Generated/**/*.cs] generated_code = true dotnet_analyzer_diagnostic.severity = none dotnet_style_qualification_for_field = false dotnet_style_qualification_for_property = false dotnet_style_qualification_for_method = false dotnet_style_qualification_for_event = false csharp_style_namespace_declarations = block_scoped dotnet_diagnostic.IDE0130.severity = none dotnet_diagnostic.IDE0046.severity = none dotnet_diagnostic.IDE0045.severity = none dotnet_diagnostic.IDE0066.severity = none dotnet_diagnostic.IDE0028.severity = none dotnet_diagnostic.CA1822.severity = none dotnet_diagnostic.IDE0290.severity = none # Disable all naming convention rules for generated code dotnet_naming_rule.constant_fields_should_be_pascal_case.severity = none dotnet_naming_rule.public_members_should_be_pascal_case.severity = none dotnet_naming_rule.private_fields_should_have_underscore_prefix.severity = noneThirdweb/Thirdweb.Contracts/ThirdwebContract.cs (1)
62-114: Nice: robust validation, checked cast, safe ABI parsing, and null-guards.This addresses prior feedback: parameter validation, checked chainId cast, ABI parse with error handling, and defensive checks on API response/result are all in place.
🧹 Nitpick comments (9)
.editorconfig (3)
11-12: Clarify “formatting” exclusion; analyzers are disabled, but formatters may still touch the file.generated_code = true and dotnet_analyzer_diagnostic.severity = none silence diagnostics, but tools like dotnet format can still reflow whitespace/EOL unless configured to skip generated code. Ensure your CI format step excludes generated files (e.g., omit --include-generated or set an exclude glob), otherwise GeneratedClient.cs may still be reformatted.
Would you like me to propose a minimal format/exclude configuration (e.g., dotnet-format args or .config) that keeps generated files untouched?
36-41: Reconsider enforcing CRLF in a cross-platform repo.For contributors on macOS/Linux and in CI, CRLF can cause churn and EOL normalization issues unless .gitattributes is tightly configured. Prefer LF or leave end_of_line unspecified and rely on .gitattributes.
Apply one of these diffs:
Option A — prefer LF:
[*.{cs,csx}] -end_of_line = crlf indent_style = space indent_size = 4 max_line_length = 200 +end_of_line = lfOption B — defer to .gitattributes (remove enforcement here):
[*.{cs,csx}] -end_of_line = crlf indent_style = space indent_size = 4 max_line_length = 200If you keep CRLF, ensure .gitattributes has an explicit rule to avoid inconsistent line endings in commits.
11-31: Exclude generated client from code coverage to mitigate the reported coverage drop.The Codecov report shows a large coverage regression; generated code is a common source of noise. Exclude GeneratedClient from coverage using runsettings or coverlet filters instead of testing it.
Suggested runsettings snippet (add to a .runsettings file and pass via dotnet test --settings):
<RunSettings> <DataCollectionRunSettings> <DataCollectors> <DataCollector friendlyName="XPlat Code Coverage"> <Configuration> <Exclude> <ModulePath>.*</ModulePath> <Source>.*Thirdweb/Thirdweb.Api/GeneratedClient\.cs</Source> </Exclude> </Configuration> </DataCollector> </DataCollectors> </DataCollectionRunSettings> </RunSettings>Alternatively, decorate the generated class with [System.Diagnostics.CodeAnalysis.ExcludeFromCodeCoverage], if your generator supports injecting attributes.
Do you want me to add a repo-level runsettings template wired into your test workflow?
Thirdweb/Thirdweb.Contracts/ThirdwebContract.cs (6)
44-45: Polish the remark for clarity and grammar.The sentence reads a bit awkwardly.
Apply this diff:
- /// This method deploys a new contract using a server wallet, create one via the ServerWallet class or api.thirdweb.com, or the dashboard. + /// This method deploys a new contract using a server wallet. Create one via the ServerWallet class, api.thirdweb.com, or the dashboard.
46-51: Doc comment exceptions don’t match actual behavior; expand and correct the list.
- The method converts JSON parsing failures into ArgumentException (with JsonException as InnerException), so documenting JsonException is misleading.
- The method can also surface OperationCanceledException from the initial API call and HTTP/api exceptions from the generated client.
Apply this diff:
- /// <exception cref="ArgumentNullException">Thrown if any of the required parameters are null.</exception> - /// <exception cref="ArgumentException">Thrown if any of the required parameters are invalid.</exception> - /// <exception cref="OverflowException">Thrown if the chain ID is too large.</exception> - /// <exception cref="InvalidOperationException">Thrown if the deployment fails or the API response is invalid.</exception> - /// <exception cref="JsonException">Thrown if the ABI JSON cannot be parsed.</exception> + /// <exception cref="ArgumentNullException">Thrown if any of the required parameters are null.</exception> + /// <exception cref="ArgumentException">Thrown if any of the required parameters are invalid, or if the ABI JSON cannot be parsed.</exception> + /// <exception cref="ArgumentOutOfRangeException">Thrown if the chain ID is too large for the current API (requires Int32).</exception> + /// <exception cref="InvalidOperationException">Thrown if the deployment fails or the API response is invalid.</exception> + /// <exception cref="OperationCanceledException">Thrown if the operation is canceled via the provided CancellationToken.</exception> + /// <exception cref="System.Net.Http.HttpRequestException">Thrown if the underlying HTTP request fails.</exception> + /// <exception cref="Thirdweb.Api.ApiException">Thrown if the Thirdweb API returns a non-success status code.</exception>Note: The code change below updates the thrown exception type for chainId overflow to match the updated docs.
73-86: Prefer IsNullOrWhiteSpace for string validation; optionally validate ETH address format.Minor improvement: treat strings with only whitespace as invalid. Optionally, validate serverWalletAddress with Nethereum’s AddressUtil.
Apply this diff:
- if (string.IsNullOrEmpty(serverWalletAddress)) + if (string.IsNullOrWhiteSpace(serverWalletAddress)) { throw new ArgumentException("Server wallet address cannot be null or empty.", nameof(serverWalletAddress)); } - if (string.IsNullOrEmpty(bytecode)) + if (string.IsNullOrWhiteSpace(bytecode)) { throw new ArgumentException("Bytecode cannot be null or empty.", nameof(bytecode)); } - if (string.IsNullOrEmpty(abi)) + if (string.IsNullOrWhiteSpace(abi)) { throw new ArgumentException("ABI cannot be null or empty.", nameof(abi)); }Optionally add ETH address format validation (requires Nethereum.Util):
if (string.IsNullOrWhiteSpace(serverWalletAddress)) { throw new ArgumentException("Server wallet address cannot be null or empty.", nameof(serverWalletAddress)); } + if (!Nethereum.Util.AddressUtil.Current.IsValidEthereumAddressHexFormat(serverWalletAddress)) + { + throw new ArgumentException("Server wallet address must be a valid 0x-prefixed Ethereum address.", nameof(serverWalletAddress)); + }
95-98: Use ArgumentOutOfRangeException and preserve the original OverflowException as InnerException.Re-throwing a new OverflowException loses the original stack and doesn’t tie the error to the offending parameter.
Apply this diff:
- catch (OverflowException) - { - throw new OverflowException($"Chain ID {chainId} is too large; ThirdwebContract.Deploy currently only supports chain IDs up to {int.MaxValue}."); - } + catch (OverflowException ex) + { + throw new ArgumentOutOfRangeException(nameof(chainId), chainId, $"Chain ID {chainId} is too large; ThirdwebContract.Deploy currently only supports chain IDs up to {int.MaxValue}.", ex); + }
104-108: Invalid ABI should be reported as ArgumentException for consistency.Returning null from deserialization indicates an invalid argument; prefer ArgumentException over InvalidOperationException.
Apply this diff:
- if (parsedAbi == null) - { - throw new InvalidOperationException("ABI deserialization returned null."); - } + if (parsedAbi == null) + { + throw new ArgumentException("ABI JSON deserialized to null.", nameof(abi)); + }
51-156: Add unit tests for ThirdwebContract.Deploy to restore patch coverageNo existing tests invoke
ThirdwebContract.Deploy(confirmed viargsearch; only commented references inThirdweb.Console/Program.cs). To recover meaningful coverage without hitting the network, add tests covering:
- Input validation exceptions:
ArgumentNullExceptionwhenclientisnullArgumentExceptionwhenchainId≤ 0ArgumentExceptionwhenserverWalletAddress,bytecode, orabiisnull/empty- Overflow check:
OverflowExceptionwhen casting a largeBigIntegerchainId >int.MaxValue- ABI parsing errors:
ArgumentExceptionon malformed JSONInvalidOperationExceptionwhen deserialized ABI isnull(e.g. passing"null")- (Optional) Happy-path without live calls:
- Inject a fake
client.Apithat returns a canned response withResult.AddressandResult.TransactionId- Abstract or mock
ThirdwebTransaction.WaitForTransactionHashandWaitForTransactionReceipt(e.g., via an interface or test-only delegate)
📜 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 (5)
.editorconfig(2 hunks)Thirdweb.Console/Program.cs(2 hunks)Thirdweb/Thirdweb.Contracts/ThirdwebContract.cs(1 hunks)Thirdweb/Thirdweb.Transactions/ThirdwebTransaction.cs(3 hunks)tw.bat(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- tw.bat
- Thirdweb/Thirdweb.Transactions/ThirdwebTransaction.cs
- Thirdweb.Console/Program.cs
🧰 Additional context used
🧬 Code Graph Analysis (1)
Thirdweb/Thirdweb.Contracts/ThirdwebContract.cs (2)
Thirdweb/Thirdweb.Transactions/ThirdwebTransaction.cs (15)
Task(47-64)Task(190-196)Task(203-208)ThirdwebTransaction(29-545)ThirdwebTransaction(35-39)ThirdwebTransaction(80-84)ThirdwebTransaction(91-95)ThirdwebTransaction(102-106)ThirdwebTransaction(113-117)ThirdwebTransaction(124-128)ThirdwebTransaction(135-139)ThirdwebTransaction(146-150)ThirdwebTransaction(157-161)ThirdwebTransaction(168-172)ThirdwebTransaction(179-183)Thirdweb/Thirdweb.Client/ThirdwebClient.cs (3)
ThirdwebClient(11-115)ThirdwebClient(34-84)ThirdwebClient(100-114)
⏰ 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 (3)
.editorconfig (2)
11-13: Good fix: generated-code path is now correct and scoped.Switching to [Thirdweb/Thirdweb.Api/GeneratedClient.cs] addresses the earlier glob/path issue and prevents suppressing analyzers for non-generated files like ThirdwebHttpClientWrapper.cs.
184-192: Reasonable severities for JSON002 and CS8981.Silencing JSON002 and downgrading CS8981 to suggestion aligns with pragmatism around string literals and lowercased identifiers used in web3 contexts.
Thirdweb/Thirdweb.Contracts/ThirdwebContract.cs (1)
115-156: LGTM: API invocation and transaction-tracking flow.
- Good use of null-coalescing after await to guard unexpected nulls.
- Validations on Address, TransactionId, Hash, and receipt wait are appropriate.
- Cancellation tokens are propagated through all async calls.
Added 'Thirdweb/Thirdweb.Api/GeneratedClient.cs' to the list of ignored files in codecov.yml to exclude it from coverage reports.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
Thirdweb/Thirdweb.Transactions/ThirdwebTransaction.cs (1)
472-520: WaitForTransactionHash: Robust polling with linked CTS and null-safety — nice improvementGood job incorporating argument validation, linked CTS timeout, and null-safe access. This directly addresses the earlier feedback.
🧹 Nitpick comments (4)
codecov.yml (2)
3-4: Good call excluding generated client and Pay from coverage; consider more robust globbingExcluding GeneratedClient.cs is appropriate to avoid noisy coverage diffs from generated code, and the Pay module exclusion aligns with the PR intent. To make the patterns future-proof and unambiguous:
- Explicitly ignore the entire Pay subtree with a trailing /**.
- Consider a pattern that future-proofs additional generated files in Thirdweb.Api, if/when NSwag emits more than a single file.
If you agree, apply one of these diffs:
Option A (minimal, explicit directory ignore):
ignore: - "Thirdweb/Thirdweb.Wallets/InAppWallet" - - "Thirdweb/Thirdweb.Pay" + - "Thirdweb/Thirdweb.Pay/**" - "Thirdweb/Thirdweb.Api/GeneratedClient.cs"Option B (future-proof generated artifacts in the same folder):
ignore: - "Thirdweb/Thirdweb.Wallets/InAppWallet" - - "Thirdweb/Thirdweb.Pay" + - "Thirdweb/Thirdweb.Pay/**" - - "Thirdweb/Thirdweb.Api/GeneratedClient.cs" + - "Thirdweb/Thirdweb.Api/GeneratedClient.cs" + - "Thirdweb/Thirdweb.Api/Generated*.cs"Please also confirm Codecov is picking up this config (root-level codecov.yml). If the ignore entries don’t reflect in the next report, we may need to verify path casing or CI working directory.
4-4: Fix: add newline at end of file (YAMLlint warning)Static analysis flagged a missing trailing newline. Add a newline to appease linters and avoid noisy diffs.
- "Thirdweb/Thirdweb.Api/GeneratedClient.cs" +Thirdweb/Thirdweb.Transactions/ThirdwebTransaction.cs (2)
265-268: Prefer parentheses around nested ternary for clarityRight-associativity makes this correct, but it’s harder to scan. Add parentheses (or expand to if/else) to avoid misreads.
Apply this diff:
- BigInteger divider = - isZkSync ? 7 - : transaction.Input.AuthorizationList == null ? 5 - : 3; + BigInteger divider = + isZkSync ? 7 + : (transaction.Input.AuthorizationList == null ? 5 : 3);
500-506: Handle empty transaction hash values, not just nullIf the API ever returns an empty string, the loop would exit early and return "", which is likely unintended. Treat empty/whitespace as “not found.”
Apply this diff:
- if (hash == null) + if (string.IsNullOrWhiteSpace(hash)) { await ThirdwebTask.Delay(100, cts.Token).ConfigureAwait(false); } - } while (hash == null && !cts.Token.IsCancellationRequested); + } while (string.IsNullOrEmpty(hash) && !cts.Token.IsCancellationRequested);Additionally, mirror this linked-CTS delay usage in WaitForTransactionReceipt to respect the internal timeout immediately during the delay:
// In WaitForTransactionReceipt: await ThirdwebTask.Delay(100, cts.Token).ConfigureAwait(false);
📜 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/Thirdweb.Transactions/ThirdwebTransaction.cs(3 hunks)codecov.yml(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
Thirdweb/Thirdweb.Transactions/ThirdwebTransaction.cs (4)
Thirdweb/Thirdweb.Client/ThirdwebClient.cs (3)
ThirdwebClient(11-115)ThirdwebClient(34-84)ThirdwebClient(100-114)Thirdweb/Thirdweb.Client/ITimeoutOptions.cs (1)
GetTimeout(14-14)Thirdweb/Thirdweb.Client/TimeoutOptions.cs (1)
GetTimeout(24-33)Thirdweb/Thirdweb.Utils/ThirdwebTask.cs (1)
ThirdwebTask(5-47)
🪛 YAMLlint (1.37.1)
codecov.yml
[error] 4-4: no new line character at the end of file
(new-line-at-end-of-file)
⏰ 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/Thirdweb.Transactions/ThirdwebTransaction.cs (1)
543-543: Trailing comma in object initializer is fineThis is a harmless style tweak that reduces future diff noise when adding fields. LGTM.
Adds the auto-generated Thirdweb.Api/GeneratedClient.cs (https://api.thirdweb.com/reference integration) and supporting nswag.json. Updates .editorconfig to exclude generated API files from linting. Removes Directory.Build.props, Directory.Packages.props, and Makefile. Updates project files to specify explicit package versions and build settings. Refactors and cleans up test and console code for consistency and style.
PR-Codex overview
This PR primarily focuses on code enhancements, including formatting adjustments, method updates, and additional API client functionality within the
ThirdwebSDK. It also includes some updates to exception handling and API response structures.Detailed summary
ThirdwebApiClientinitialization inThirdwebClient.tw.bat.IThirdwebHttpClientinterface methods to include access modifiers.JsonSerializersettings in various methods for better null handling.WaitForTransactionHashinThirdwebTransactionfor polling transaction hashes.Summary by CodeRabbit
New Features
Refactor
Chores