Conversation
… approach to check through the UI instead of API call.
…ared transaction id
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Additional Comments (1)
-
apps/wallets/quickstart-devkit/e2e/e2e.spec.ts, line 64-72 (link)logic: Recipient address logic is incomplete - missing handling for Stellar chain
6 files reviewed, 3 comments
There was a problem hiding this comment.
Additional Comments (1)
-
apps/wallets/quickstart-devkit/e2e/e2e.spec.ts, line 66-74 (link)logic: Redundant logic in else block -
walletAddress.startsWith("0x")checked twice, and Stellar chain not handledThe else block (lines 71-73) repeats the
walletAddress.startsWith("0x")check that already happened at line 66. This means Stellar wallets will fall through to the else block and be handled inconsistently.
6 files reviewed, 1 comment
There was a problem hiding this comment.
Additional Comments (1)
-
apps/wallets/quickstart-devkit/components/transfer.tsx, line 64-73 (link)logic: Label displays "USDC" but radio value is "usdxm" - confusing for non-Stellar chains
For EVM chains, the UI should show and send USDC, not USDXM. Only Stellar uses USDXM.
8 files reviewed, 2 comments
Additional Comments (5)
Prompt To Fix With AIThis is a comment left during a code review.
Path: apps/wallets/quickstart-devkit/components/transfer.tsx
Line: 64:72
Comment:
Label shows "USDC" but token value is "usdxm" - mismatch between UI and actual token
```suggestion
<label className="flex items-center gap-2 cursor-pointer">
<input
type="radio"
name="usdc"
className="h-4 w-4"
checked={token === "usdxm"}
onChange={() => setToken("usdxm")}
/>
<span>USDXM</span>
</label>
```
How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix With AIThis is a comment left during a code review.
Path: apps/wallets/quickstart-devkit/components/transfer.tsx
Line: 182:190
Comment:
Same issue - label shows "USDC" but token value is "usdxm"
```suggestion
<label className="flex items-center gap-2 cursor-pointer">
<input
type="radio"
name="token"
className="h-4 w-4"
checked={token === "usdxm"}
onChange={() => setToken("usdxm")}
/>
<span>USDXM</span>
</label>
```
How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix With AIThis is a comment left during a code review.
Path: apps/wallets/quickstart-devkit/e2e/helpers/auth.ts
Line: 500:501
Comment:
Indentation broken - this line is inside try block but not indented
```suggestion
try {
const url = new URL(page.url());
```
How can I resolve this? If you propose a fix, please make it concise.
export function getStellarAlias(signerType: SignerType): string {
let randomSuffix = randomSuffixCache.get(signerType);
if (randomSuffix === undefined) {
randomSuffix = Math.floor(Math.random() * 1000000);
randomSuffixCache.set(signerType, randomSuffix);
}
return `stellartestingwallet${randomSuffix}`;
}Prompt To Fix With AIThis is a comment left during a code review.
Path: apps/wallets/quickstart-devkit/e2e/config/constants.ts
Line: 285:295
Comment:
Nullish coalescing fallback can generate different suffixes - if `randomSuffixCache.get(signerType)` returns undefined, a new random number is generated but not guaranteed to be set before this function is called again. Consider simplifying:
```typescript
export function getStellarAlias(signerType: SignerType): string {
let randomSuffix = randomSuffixCache.get(signerType);
if (randomSuffix === undefined) {
randomSuffix = Math.floor(Math.random() * 1000000);
randomSuffixCache.set(signerType, randomSuffix);
}
return `stellartestingwallet${randomSuffix}`;
}
```
How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix With AIThis is a comment left during a code review.
Path: apps/wallets/quickstart-devkit/e2e/e2e.spec.ts
Line: 483:483
Comment:
Regex only allows 1-2 decimal places, but transfers use 0.0001 (4 decimals) - assertion will fail
```suggestion
expectAuth(balances.nativeToken.amount).toMatch(/^\d+(\.\d+)?$/);
```
How can I resolve this? If you propose a fix, please make it concise. |
albertoelias-crossmint
left a comment
albertoelias-crossmint
left a comment
There was a problem hiding this comment.
looks good, left some comments
|
|
||
| // Generate a unique alias for Stellar wallets based on the email's random suffix | ||
| // This ensures consistency - same email = same alias | ||
| export function getStellarAlias(signerType: SignerType): string { |
There was a problem hiding this comment.
why do we only generate alias for stellar?
There was a problem hiding this comment.
Stellar requires unique aliases when using random emails to avoid wallet conflicts. Other chains (EVM, Solana) don't require aliases for tests. While working on that, I got some random failures, which is why I added this logic.
There was a problem hiding this comment.
can you explain further? it should be the same for all chains
There was a problem hiding this comment.
While testing, I observed that randomly, the stellar test cases were failing with the error.
And when I was looking for the root cause of that, I changed the stellar address, and it worked, so after implementing this logic for stellar addresses, it worked every time.
To reproduce locally, you can:
- Run the application
- Then open the following link Stellar Wallet
- It will redirect you to the login page from the Stellar address
- Login using the mailosaur address, which can be anything@gljzc5s9.mailosaur.net
- Then open another incognito tab from a new session.
- Open the same link and try to log in again
- Observe now that the error is displayed.
It only happened with the Stellar address. Is this a bug?
There was a problem hiding this comment.
@alberto-crossmint do aliases behave differently in stellar?
There was a problem hiding this comment.
They don't. Let me check
There was a problem hiding this comment.
Answered in slack!
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Additional Comments (1)
Prompt To Fix With AIThis is a comment left during a code review.
Path: apps/wallets/quickstart-devkit/e2e/helpers/wallet.ts
Line: 76:76
Comment:
Console log message says "Failed" but uses success emoji ✅ - should use ❌ or ⚠️
```suggestion
console.log("⚠️ Failed to get wallet balance, retrying...");
```
How can I resolve this? If you propose a fix, please make it concise. |
| } | ||
|
|
||
| const baseAlias = SIGNER_EMAIL_BASE[signerType]; | ||
| // Generate a random number between 0 and 999999 to create unique email addresses |
There was a problem hiding this comment.
Nit: we are creating random aliases not random email addresses, no?
| // This ensures consistency - same email = same alias | ||
| export function getStellarAlias(signerType: SignerType): string { | ||
| // Get the random suffix that was used for the email | ||
| const randomSuffix = randomSuffixCache.get(signerType) ?? Math.floor(Math.random() * 1000000); |
There was a problem hiding this comment.
I don't get this, so we'll be using the same alias for the same signer type? What's the motivation?
3 participants
Description
PS: This PR has all the changes from the smoke tests branch #1549
Test plan
All tests should work
Package updates