fix: Add validation for getContract params #5780

joaquim-verges · 2024-12-17T21:13:04Z

Problem solved

Fixes TOOL-2768

PR-Codex overview

This PR focuses on adding validation to the getContract function in the thirdweb package, ensuring that the provided client, address, and chain are valid before proceeding.

Detailed summary

Added validation checks in the getContract function:
- Validates if options.client is provided.
- Validates if options.address is a valid address using isAddress.
- Validates if options.chain and options.chain.id are provided.

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

changeset-bot · 2024-12-17T21:13:08Z

🦋 Changeset detected

Latest commit: 4be5192

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
thirdweb	Patch
@thirdweb-dev/wagmi-adapter	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

vercel · 2024-12-17T21:13:08Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
docs-v2	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Dec 18, 2024 9:31pm
thirdweb_playground	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Dec 18, 2024 9:31pm
thirdweb-www	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Dec 18, 2024 9:31pm
wallet-ui	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Dec 18, 2024 9:31pm

joaquim-verges · 2024-12-17T21:13:21Z

fix: Add validation for getContract params #5780 👈 (View in Graphite)
main

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

merge-queue - adds this PR to the back of the merge queue
hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

_{An organization admin has enabled the Graphite Merge Queue in this repository.} _{Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.}

This stack of pull requests is managed by Graphite. Learn more about stacking.

graphite-app · 2024-12-17T21:13:45Z

packages/thirdweb/src/contract/contract.ts

+  if (!isAddress(options.address)) {
+    throw new Error("getContract validation error: Invalid address");
+  }
+  if (!options.chain || !options.chain.id) {


The check !options.chain.id will evaluate to true when chain.id is 0, which is a valid chain ID. Consider using typeof options.chain.id !== 'number' to properly validate the type while allowing zero values.

Spotted by Graphite Reviewer

Is this helpful? React 👍 or 👎 to let us know.

linear · 2024-12-17T21:15:20Z

TOOL-2768 SDK should throw clear error when creating a contract with undefined address

github-actions · 2024-12-17T21:16:31Z

size-limit report 📦

Path	Size	Loading time (3g)	Running time (snapdragon)	Total time
thirdweb (esm)	45.02 KB (+0.2% 🔺)	901 ms (+0.2% 🔺)	2.8 s (+3.03% 🔺)	3.7 s
thirdweb (cjs)	110.76 KB (-0.08% 🔽)	2.3 s (-0.08% 🔽)	7.5 s (+0.41% 🔺)	9.7 s
thirdweb (minimal + tree-shaking)	5.58 KB (0%)	112 ms (0%)	492 ms (+130.7% 🔺)	603 ms
thirdweb/chains (tree-shaking)	506 B (0%)	10 ms (0%)	43 ms (-52.49% 🔽)	53 ms
thirdweb/react (minimal + tree-shaking)	19.1 KB (+0.19% 🔺)	383 ms (+0.19% 🔺)	863 ms (-12.47% 🔽)	1.3 s

codecov · 2024-12-17T23:54:28Z

Codecov Report

Attention: Patch coverage is 0% with 15 lines in your changes missing coverage. Please review.

Project coverage is 53.01%. Comparing base (07ad025) to head (4be5192).
Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
packages/thirdweb/src/contract/contract.ts	0.00%	15 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5780      +/-   ##
==========================================
- Coverage   53.02%   53.01%   -0.02%     
==========================================
  Files        1101     1101              
  Lines       59080    59096      +16     
  Branches     4809     4813       +4     
==========================================
+ Hits        31328    31329       +1     
- Misses      27034    27049      +15     
  Partials      718      718

Flag	Coverage Δ		*Carryforward flag
legacy_packages	`65.68% <ø> (ø)`		Carriedforward from 07ad025
packages	`50.15% <0.00%> (-0.02%)`	⬇️

*This pull request uses carry forward flags. Click here to find out more.

Files with missing lines	Coverage Δ
packages/thirdweb/src/contract/contract.ts	`28.57% <0.00%> (-71.43%)`	⬇️

joaquim-verges · 2024-12-18T21:01:55Z

Merge activity

Dec 18, 4:01 PM EST: Graphite disabled "merge when ready" on this PR due to: a merge conflict with the target branch; resolve the conflict and try again..

DASH-540 * on the chat page -> move the container 1 level down - so the chat can be scrolled from the corner of the page as well * reduce container max-width for the history page to 800px to match it with landing page * stop the auto scroll if user interacts with chat UI * custom 404 page for the nebula subdomain * remove the announcement banner from the 404 pages ( this affects entire dashboard) * fix empty title in chat history page * fix overflow in chat history page on mobile when there's a long text without a whitespace  --- This PR focuses on enhancing the chat functionality and user experience in the `Nebula` application. It introduces auto-scrolling features, improves layout styling, and adds a new `404 Not Found` page. - Added auto-scrolling feature to `Chats` component. - Introduced `setEnableAutoScroll` function to manage scrolling behavior. - Enhanced styling in `ChatHistoryPage` and `SessionCard`. - Implemented a new `NebulaNotFound` component with a user-friendly 404 message. - Updated `ChatPageContent` to utilize the new auto-scroll functionality. > ✨ Ask PR-Codex anything about this PR by commenting with `/codex {your question}`

github-actions bot assigned joaquim-verges Dec 17, 2024

github-actions bot added packages SDK Involves changes to the thirdweb SDK labels Dec 17, 2024

joaquim-verges marked this pull request as ready for review December 17, 2024 21:13

joaquim-verges requested review from gregfromstl and jnsdls as code owners December 17, 2024 21:13

graphite-app bot reviewed Dec 17, 2024

View reviewed changes

vercel bot deployed to Preview – wallet-ui December 17, 2024 21:16 View deployment

vercel bot deployed to Preview – thirdweb_playground December 17, 2024 21:16 View deployment

vercel bot deployed to Preview – docs-v2 December 17, 2024 21:21 View deployment

vercel bot deployed to Preview – thirdweb-www December 17, 2024 21:24 View deployment

gregfromstl approved these changes Dec 17, 2024

View reviewed changes

joaquim-verges requested review from a team as code owners December 17, 2024 22:19

vercel bot deployed to Preview – thirdweb_playground December 17, 2024 22:20 View deployment

vercel bot deployed to Preview – wallet-ui December 17, 2024 22:21 View deployment

vercel bot deployed to Preview – docs-v2 December 17, 2024 22:21 View deployment

vercel bot deployed to Preview – thirdweb-www December 17, 2024 22:28 View deployment

joaquim-verges force-pushed the fix_Add_validation_for_getContract_params branch from 520487c to b740f94 Compare December 17, 2024 23:25

vercel bot deployed to Preview – thirdweb_playground December 17, 2024 23:28 View deployment

vercel bot deployed to Preview – wallet-ui December 17, 2024 23:28 View deployment

vercel bot deployed to Preview – docs-v2 December 17, 2024 23:33 View deployment

vercel bot deployed to Preview – thirdweb-www December 17, 2024 23:33 View deployment

joaquim-verges force-pushed the fix_Add_validation_for_getContract_params branch from b740f94 to 4be5192 Compare December 18, 2024 21:20

vercel bot deployed to Preview – wallet-ui December 18, 2024 21:23 View deployment

vercel bot deployed to Preview – thirdweb_playground December 18, 2024 21:24 View deployment

vercel bot deployed to Preview – thirdweb-www December 18, 2024 21:30 View deployment

vercel bot deployed to Preview – docs-v2 December 18, 2024 21:31 View deployment

joaquim-verges merged commit a2a61df into main Dec 18, 2024
31 of 32 checks passed

joaquim-verges deleted the fix_Add_validation_for_getContract_params branch December 18, 2024 23:53

jnsdls mentioned this pull request Dec 18, 2024

Version Packages #5785

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Add validation for getContract params #5780

fix: Add validation for getContract params #5780

joaquim-verges commented Dec 17, 2024 •

edited by pr-codex bot

Loading

changeset-bot bot commented Dec 17, 2024 •

edited

Loading

vercel bot commented Dec 17, 2024 •

edited

Loading

joaquim-verges commented Dec 17, 2024

graphite-app bot Dec 17, 2024

linear bot commented Dec 17, 2024

github-actions bot commented Dec 17, 2024 •

edited

Loading

codecov bot commented Dec 17, 2024 •

edited

Loading

joaquim-verges commented Dec 18, 2024

fix: Add validation for getContract params #5780

fix: Add validation for getContract params #5780

Conversation

joaquim-verges commented Dec 17, 2024 • edited by pr-codex bot Loading

Problem solved

PR-Codex overview

Detailed summary

changeset-bot bot commented Dec 17, 2024 • edited Loading

🦋 Changeset detected

vercel bot commented Dec 17, 2024 • edited Loading

joaquim-verges commented Dec 17, 2024

How to use the Graphite Merge Queue

graphite-app bot Dec 17, 2024

Choose a reason for hiding this comment

linear bot commented Dec 17, 2024

github-actions bot commented Dec 17, 2024 • edited Loading

size-limit report 📦

codecov bot commented Dec 17, 2024 • edited Loading

Codecov Report

joaquim-verges commented Dec 18, 2024

Merge activity

joaquim-verges commented Dec 17, 2024 •

edited by pr-codex bot

Loading

changeset-bot bot commented Dec 17, 2024 •

edited

Loading

vercel bot commented Dec 17, 2024 •

edited

Loading

github-actions bot commented Dec 17, 2024 •

edited

Loading

codecov bot commented Dec 17, 2024 •

edited

Loading