Tt 1342/address resolver documentation#6
Conversation
- Changed references from TradeTrust to TrustVC throughout the document. - Updated example links and images to reflect TrustVC branding. - Clarified address book column names and import process. - Added new images for TrustVC's address resolver features.
- Updated the description of the third-party address resolver to clarify its functionality. - Simplified the setup instructions for creating a resolver endpoint and hosting options. - Added supported JSON response formats for better guidance on implementation. - Improved clarity and structure of the configuration steps in the settings page.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis update replaces TradeTrust-specific address-resolver documentation with TrustVC-specific instructions. Local Address Book field requirements change from Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
- Added a note on securing the resolver endpoint for production use. - Included instructions for configuring API headers and keys. - Clarified the description of the endpoint input with an example URL.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/how-tos/advanced/address-resolver.md (1)
74-81:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix address case-normalization bug in the local matching example.
Your CSV import stores keys in lowercase (
addressBook[addressText.toLowerCase()]), but the lookup comparesaddressToMatch === keywithout normalizingaddressToMatch. If a user copies a checksummed/mixed-case address, this example may not resolve even when the address exists.✅ Suggested fix
const addressToMatch = "0xabc..."; // your local address +const normalized = addressToMatch.toLowerCase(); for (const [key, value] of Object.entries(addressBook)) { - if (addressToMatch === key) { + if (normalized === key) { return value; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/how-tos/advanced/address-resolver.md` around lines 74 - 81, The example's lookup compares addressToMatch to keys in addressBook that were stored lowercased (addressBook[addressText.toLowerCase()]), so normalize the incoming address before matching: convert addressToMatch to lowercase (or compare key.toLowerCase()) when iterating over addressBook so checksummed/mixed-case addresses resolve; update the snippet referencing addressToMatch and addressBook accordingly to use .toLowerCase() in the comparison.
🧹 Nitpick comments (3)
docs/how-tos/advanced/address-resolver.md (3)
19-32: ⚡ Quick winEditorial: capitalize “Excel” consistently and improve terminology (“3rd-party”).
Static analysis hints suggest several instances of “csv/excel” where “Excel” should be capitalized (Line ~19, ~26, ~32, ~72). Also, “3rd party” appears in a header area and may be better as “3rd-party” for clarity/consistency.
These are low-risk, but will improve readability and reduce doc churn.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/how-tos/advanced/address-resolver.md` around lines 19 - 32, Update the doc text to consistently capitalize "Excel" everywhere it appears (replace occurrences of "csv/excel", "csv/Excel", etc., with "CSV/Excel" or "CSV and Excel" as appropriate) and standardize the 3rd-party phrasing by replacing "3rd party" with "3rd-party" throughout the file; focus changes around the strings "csv/excel", "Excel", and "3rd party" in the Address Book/Setup sections (and any similar headings) to maintain consistent terminology and capitalization.
54-67: ⚡ Quick winAlign “required columns” with the parsing snippet.
The doc says the minimal required columns are exactly
NameandAddress(Line 19-23), but the parsing example accepts several alternate header names (row.name,row.Identifier,row.identifier). That’s not necessarily wrong, but it’s undocumented and can confuse implementers trying to stick to the stated “minimal required” spec.Either:
- tighten the example to only use
row.Nameandrow.Address, or- update the prose to explicitly say the import supports these alternate header variants.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/how-tos/advanced/address-resolver.md` around lines 54 - 67, The parsing example in csvToAddressBook accepts alternate header names (row.name, row.Identifier, row.identifier, row.Address) but the prose declares only Name and Address as required; either make the parser and example strict to only use row.Name and row.Address or update the documentation to list the supported header variants. To fix, choose one approach: 1) tighten the example by changing the parser logic and references in csvToAddressBook (and any mention of parse) to only use row.Name and row.Address and remove alternates from the example, or 2) update the doc text to explicitly state that csvToAddressBook recognizes Name/name/Identifier/identifier for the identifier column and Address/address for the address column and show the existing code as the canonical example (mentioning addressBook and the forEach handling of row). Ensure the chosen change keeps the parse(...) call behavior and example consistent with the written “minimal required columns” statement.
98-129: ⚡ Quick winMake JSON key casing explicit for third-party resolver responses.
Third-party resolver response examples use lowercase keys (
name,address) (Line 100-102, 105-129), while local address book usesName/Address(Line 21-22). Please add a one-line note stating the third-party endpoint must usename/address(lowercase) exactly as documented, to prevent copy/paste errors.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/how-tos/advanced/address-resolver.md` around lines 98 - 129, Add a one-line note under the "Supported response shapes" examples making explicit that third-party resolver endpoints must use the lowercase JSON keys "name" and "address" exactly (not "Name"/"Address"), so consumers copying those examples won't mismatch casing; place this sentence near the JSON examples (the block showing array and the nested "entries"/"results" objects) to clarify the required key names.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/how-tos/advanced/address-resolver.md`:
- Around line 137-140: Add a security notice to the "resolver settings" section
warning that API headers/keys (e.g., `x-api-key`) configured via the Settings
page are used by the TrustVC web app in the browser and therefore will be
exposed in network requests and browser storage; explicitly state these should
not be treated as highly sensitive production secrets and recommend implementing
a server-side proxy to make resolver calls (describe proxy as the recommended
approach for API key rotation and endpoint protection) and show that the
alternative is using short-lived tokens or server-hosted credentials for
production.
---
Outside diff comments:
In `@docs/how-tos/advanced/address-resolver.md`:
- Around line 74-81: The example's lookup compares addressToMatch to keys in
addressBook that were stored lowercased
(addressBook[addressText.toLowerCase()]), so normalize the incoming address
before matching: convert addressToMatch to lowercase (or compare
key.toLowerCase()) when iterating over addressBook so checksummed/mixed-case
addresses resolve; update the snippet referencing addressToMatch and addressBook
accordingly to use .toLowerCase() in the comparison.
---
Nitpick comments:
In `@docs/how-tos/advanced/address-resolver.md`:
- Around line 19-32: Update the doc text to consistently capitalize "Excel"
everywhere it appears (replace occurrences of "csv/excel", "csv/Excel", etc.,
with "CSV/Excel" or "CSV and Excel" as appropriate) and standardize the
3rd-party phrasing by replacing "3rd party" with "3rd-party" throughout the
file; focus changes around the strings "csv/excel", "Excel", and "3rd party" in
the Address Book/Setup sections (and any similar headings) to maintain
consistent terminology and capitalization.
- Around line 54-67: The parsing example in csvToAddressBook accepts alternate
header names (row.name, row.Identifier, row.identifier, row.Address) but the
prose declares only Name and Address as required; either make the parser and
example strict to only use row.Name and row.Address or update the documentation
to list the supported header variants. To fix, choose one approach: 1) tighten
the example by changing the parser logic and references in csvToAddressBook (and
any mention of parse) to only use row.Name and row.Address and remove alternates
from the example, or 2) update the doc text to explicitly state that
csvToAddressBook recognizes Name/name/Identifier/identifier for the identifier
column and Address/address for the address column and show the existing code as
the canonical example (mentioning addressBook and the forEach handling of row).
Ensure the chosen change keeps the parse(...) call behavior and example
consistent with the written “minimal required columns” statement.
- Around line 98-129: Add a one-line note under the "Supported response shapes"
examples making explicit that third-party resolver endpoints must use the
lowercase JSON keys "name" and "address" exactly (not "Name"/"Address"), so
consumers copying those examples won't mismatch casing; place this sentence near
the JSON examples (the block showing array and the nested "entries"/"results"
objects) to clarify the required key names.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 900cafa2-1aae-42d5-9d6e-fea33dde901f
⛔ Files ignored due to path filters (16)
static/docs/reference/tradetrust-website/address-resolved.pngis excluded by!**/*.pngstatic/docs/reference/tradetrust-website/local-csv.pngis excluded by!**/*.pngstatic/docs/reference/tradetrust-website/qrcode.pngis excluded by!**/*.pngstatic/docs/reference/tradetrust-website/return-search.pngis excluded by!**/*.pngstatic/docs/reference/trustvc-website/address-resolved.pngis excluded by!**/*.pngstatic/docs/reference/trustvc-website/address-resolver-empty-form.pngis excluded by!**/*.pngstatic/docs/reference/trustvc-website/address-resolver-filled-form.pngis excluded by!**/*.pngstatic/docs/reference/trustvc-website/api-gateway.pngis excluded by!**/*.pngstatic/docs/reference/trustvc-website/create-key.pngis excluded by!**/*.pngstatic/docs/reference/trustvc-website/create-project.pngis excluded by!**/*.pngstatic/docs/reference/trustvc-website/enable-api.pngis excluded by!**/*.pngstatic/docs/reference/trustvc-website/local-csv.pngis excluded by!**/*.pngstatic/docs/reference/trustvc-website/return-search.pngis excluded by!**/*.pngstatic/docs/reference/trustvc-website/route53.pngis excluded by!**/*.pngstatic/docs/reference/trustvc-website/settings-address-book.pngis excluded by!**/*.pngstatic/docs/reference/trustvc-website/settings-address-book1.pngis excluded by!**/*.png
📒 Files selected for processing (1)
docs/how-tos/advanced/address-resolver.md
| For production, secure your resolver endpoint. | ||
|
|
||
| - If your endpoint is protected, configure an API header and API key in resolver settings (for example `x-api-key` + your token). | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
git ls-files | head -20
find . -maxdepth 2 -type f -name "*.md" | grep -i resolverRepository: TrustVC/TrustVC-Documentation
Length of output: 806
🏁 Script executed:
cat -n docs/how-tos/advanced/address-resolver.mdRepository: TrustVC/TrustVC-Documentation
Length of output: 8320
🏁 Script executed:
rg -i "resolver" --type ts --type js --type tsx --type jsx -A 5 -B 2 | head -100
rg -i "address.*resolver\|resolver.*address" --type md | head -50Repository: TrustVC/TrustVC-Documentation
Length of output: 101
🏁 Script executed:
rg -i "resolver" --type md -A 5 -B 2 | head -150
grep -r "github.com/TrustVC" . --include="*.md"Repository: TrustVC/TrustVC-Documentation
Length of output: 11261
🏁 Script executed:
cat docs/common-issues/cors-error.mdRepository: TrustVC/TrustVC-Documentation
Length of output: 3035
🏁 Script executed:
rg -i "fetch\|http\|request\|backend\|server.*side\|client.*side" docs/how-tos/advanced/address-resolver.md
rg -i "trustvc.*application\|web.*application" docs/how-tos/advanced/ -A 3 -B 3Repository: TrustVC/TrustVC-Documentation
Length of output: 55
🏁 Script executed:
rg -i "production\|security\|exposed\|browser\|frontend\|backend" docs/how-tos/advanced/address-resolver.md
rg -i "settings.*page\|website.*application" docs/how-tos/advanced/address-resolver.md -B 3 -A 3Repository: TrustVC/TrustVC-Documentation
Length of output: 55
🏁 Script executed:
sed -n '130,180p' docs/how-tos/advanced/address-resolver.mdRepository: TrustVC/TrustVC-Documentation
Length of output: 1871
Add security warning for API keys configured in browser-based settings.
The documentation instructs users to configure API headers and keys for protected resolver endpoints (lines 137-140), but does not warn that the TrustVC website application runs verification in the browser. This means resolver endpoint calls with the configured API key would be made from the frontend, exposing sensitive credentials in network requests visible via DevTools and browser storage.
Add a security notice cautioning users that API keys configured in the Settings page should not be treated as highly sensitive production secrets, or recommend implementing a backend proxy to make resolver calls server-side instead. If API key rotation or endpoint protection is critical, document the proxy approach as the recommended method.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/how-tos/advanced/address-resolver.md` around lines 137 - 140, Add a
security notice to the "resolver settings" section warning that API headers/keys
(e.g., `x-api-key`) configured via the Settings page are used by the TrustVC web
app in the browser and therefore will be exposed in network requests and browser
storage; explicitly state these should not be treated as highly sensitive
production secrets and recommend implementing a server-side proxy to make
resolver calls (describe proxy as the recommended approach for API key rotation
and endpoint protection) and show that the alternative is using short-lived
tokens or server-hosted credentials for production.
- Included a security notice about the exposure of API headers/keys in the TrustVC web app. - Recommended using a server-side proxy for resolver calls to protect sensitive credentials. - Suggested alternatives for environments where a proxy cannot be used, such as short-lived tokens.
…PI keys - Deleted the security notice regarding the exposure of API headers/keys in the TrustVC web app. - Removed recommendations for using a server-side proxy and alternatives for environments without a proxy.
Summary by CodeRabbit
Documentation