feat: add jetton management functionality including metadata resoluti…#118
Open
nikdim03 wants to merge 5 commits into
Open
feat: add jetton management functionality including metadata resoluti…#118nikdim03 wants to merge 5 commits into
nikdim03 wants to merge 5 commits into
Conversation
…on and address validation
📝 WalkthroughWalkthroughThis PR introduces a new ChangesJettons API and Gasless Send UI
DeFi Bridge Error Standardization
API Code Generation: x-generic-instance-type Support
Sequence Diagram(s)sequenceDiagram
rect rgba(173, 216, 230, 0.5)
Note over SendTransactionScreen,ITONJettonsManager: Fee-asset metadata loading
SendTransactionScreen->>SendTokensViewModel: loadFeeAssetMetadata() [LaunchedEffect on feeAssets.size]
SendTokensViewModel->>ITONJettonsManager: jettonInfo(feeAssetAddress, network)
ITONJettonsManager-->>SendTokensViewModel: TONJettonInfo (symbol, decimals, icon)
SendTokensViewModel->>SendTokensViewModel: resolvedFeeAssets updated, _state.feeAssets refreshed
end
rect rgba(255, 220, 180, 0.5)
Note over SendTransactionScreen,GaslessRelay: Gasless max computation
SendTransactionScreen->>SendTokensViewModel: useMax() [gasless enabled]
SendTokensViewModel->>GaslessRelay: quote(probeAmount = balance/2) to determine feeRaw
GaslessRelay-->>SendTokensViewModel: feeRaw
SendTokensViewModel->>SendTokensViewModel: amount = (balanceRaw - feeRaw), scheduleQuote()
end
rect rgba(200, 255, 200, 0.5)
Note over SendTransactionScreen,SendTokensViewModel: Post-send acknowledgement
SendTransactionScreen->>SendTokensViewModel: acknowledgeSent() [on state.sent == true]
SendTokensViewModel->>SendTokensViewModel: sent flag cleared
SendTransactionScreen->>SendTransactionScreen: onBack()
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Contributor
|
@nikdim03 Where is new TONJettonInfo models? I don't see it in PR |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
AndroidDemo/app/src/main/java/io/ton/walletkit/demo/presentation/ui/screen/SendTransactionScreen.kt (1)
278-282: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueConsider using a more explicit disabled state for the switch.
The disabled switch (line 281) uses
checked = falsewith an emptyonCheckedChange = {}lambda and visual dimming viaalpha(0.4f). While functional, this pattern may not clearly convey the disabled state to accessibility services or future maintainers. Consider whetherTonSwitchsupports an explicitenabledparameter or a semantic disabled state.♻️ Possible improvement if TonSwitch supports enabled parameter
TonSwitch( checked = state.gaslessEnabled, onCheckedChange = viewModel::setGaslessEnabled, enabled = state.gaslessSupported )If
TonSwitchdoes not have anenabledparameter, the current approach is acceptable.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@AndroidDemo/app/src/main/java/io/ton/walletkit/demo/presentation/ui/screen/SendTransactionScreen.kt` around lines 278 - 282, The disabled TonSwitch in the else branch uses an empty onCheckedChange lambda and alpha modifier to convey disabled state, which may not be clear to accessibility services. Check if the TonSwitch component supports an explicit enabled parameter. If it does, refactor the else branch to remove the conditional logic and instead add enabled = state.gaslessSupported directly to the TonSwitch call, along with onCheckedChange = viewModel::setGaslessEnabled and checked = state.gaslessEnabled in both branches. This will provide explicit semantic information about the disabled state rather than relying on visual dimming.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@AndroidDemo/app/src/main/java/io/ton/walletkit/demo/presentation/viewmodel/SendTokensViewModel.kt`:
- Around line 361-399: In the computeGaslessMax function, the fee parsing from
quote().fee is using toBigIntegerOrNull() with a silent fallback to
BigInteger.ZERO via the elvis operator, which masks parsing failures and causes
incorrect maxRaw calculation when the fee cannot be parsed. Replace the silent
fallback with explicit error handling that throws an exception or sets an error
state when the fee string cannot be parsed to BigInteger, ensuring that
malformed fee responses from the relay fail explicitly rather than allowing the
user to incorrectly send their entire balance without accounting for the fee.
- Around line 288-323: The loadFeeAssetMetadata function launches multiple
concurrent coroutines that each write to the shared resolvedFeeAssets mutable
map without synchronization, creating a race condition. Replace the
initialization of the resolvedFeeAssets field (currently mutableMapOf()) with
ConcurrentHashMap() to provide thread-safe concurrent access, and add the import
statement for java.util.concurrent.ConcurrentHashMap at the top of the file.
In
`@TONWalletKit-Android/impl/src/main/java/io/ton/walletkit/engine/operations/responses/BridgeResponses.kt`:
- Around line 100-101: The ValidateJettonAddressResponse data class has a
default value of false assigned to the valid field, which can mask protocol
failures when the field is unexpectedly omitted from the response. Remove the
default value assignment from the valid field in ValidateJettonAddressResponse
to make it a required field, ensuring that deserialization fails clearly if the
JS bridge omits the valid field rather than silently defaulting to false.
---
Nitpick comments:
In
`@AndroidDemo/app/src/main/java/io/ton/walletkit/demo/presentation/ui/screen/SendTransactionScreen.kt`:
- Around line 278-282: The disabled TonSwitch in the else branch uses an empty
onCheckedChange lambda and alpha modifier to convey disabled state, which may
not be clear to accessibility services. Check if the TonSwitch component
supports an explicit enabled parameter. If it does, refactor the else branch to
remove the conditional logic and instead add enabled = state.gaslessSupported
directly to the TonSwitch call, along with onCheckedChange =
viewModel::setGaslessEnabled and checked = state.gaslessEnabled in both
branches. This will provide explicit semantic information about the disabled
state rather than relying on visual dimming.
🪄 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: edddcf25-1db8-4b52-88b4-5112781c7b1c
📒 Files selected for processing (15)
AndroidDemo/app/src/main/java/io/ton/walletkit/demo/presentation/ui/screen/SendTransactionScreen.ktAndroidDemo/app/src/main/java/io/ton/walletkit/demo/presentation/viewmodel/SendTokensViewModel.ktScripts/generate-api/generate-api-models.shScripts/generate-api/templates/modelGeneric.mustacheTONWalletKit-Android/api/src/main/java/io/ton/walletkit/ITONWalletKit.ktTONWalletKit-Android/api/src/main/java/io/ton/walletkit/jettons/ITONJettonsManager.ktTONWalletKit-Android/impl/src/main/assets/walletkit/walletkit-android-bridge.mjsTONWalletKit-Android/impl/src/main/java/io/ton/walletkit/core/TONWalletKit.ktTONWalletKit-Android/impl/src/main/java/io/ton/walletkit/engine/WalletKitEngine.ktTONWalletKit-Android/impl/src/main/java/io/ton/walletkit/engine/WebViewWalletKitEngine.ktTONWalletKit-Android/impl/src/main/java/io/ton/walletkit/engine/operations/JettonsOperations.ktTONWalletKit-Android/impl/src/main/java/io/ton/walletkit/engine/operations/requests/JettonsRequests.ktTONWalletKit-Android/impl/src/main/java/io/ton/walletkit/engine/operations/responses/BridgeResponses.ktTONWalletKit-Android/impl/src/main/java/io/ton/walletkit/internal/constants/BridgeMethodConstants.ktTONWalletKit-Android/impl/src/main/java/io/ton/walletkit/jettons/TONJettonsManager.kt
💤 Files with no reviewable changes (1)
- Scripts/generate-api/generate-api-models.sh
Comment on lines
+288
to
+323
| /** | ||
| * Resolve fee-asset jetton metadata (ticker / decimals / icon) via [ITONWalletKit.jettons]. | ||
| */ | ||
| fun loadFeeAssetMetadata() { | ||
| val pending = _state.value.feeAssets.filter { it.address !in resolvedFeeAssets } | ||
| if (pending.isEmpty()) return | ||
| viewModelScope.launch { | ||
| val manager = kit.jettons() | ||
| val network = wallet.network() | ||
| pending.forEach { asset -> | ||
| launch { | ||
| runCatching { | ||
| val info = manager.jettonInfo(TONUserFriendlyAddress(asset.address), network) | ||
| ?: return@launch | ||
| val symbol = info.symbol.ifEmpty { info.name }.ifEmpty { asset.symbol } | ||
| val resolved = asset.copy( | ||
| symbol = symbol, | ||
| decimals = info.decimals ?: asset.decimals, | ||
| imageSource = info.image ?: asset.imageSource, | ||
| ) | ||
| resolvedFeeAssets[resolved.address] = resolved | ||
| _state.update { state -> | ||
| state.copy( | ||
| feeAssets = state.feeAssets.map { if (it.address == resolved.address) resolved else it }, | ||
| selectedFeeAsset = state.selectedFeeAsset | ||
| ?.takeIf { it.address == resolved.address } | ||
| ?.let { resolved } | ||
| ?: state.selectedFeeAsset, | ||
| ) | ||
| } | ||
| }.onFailure { Log.e(TAG, "Failed to load jetton info for ${asset.address}", it) } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Race condition on mutable map from concurrent coroutines.
Multiple coroutines are launched in parallel (line 298) and each writes to the shared resolvedFeeAssets mutable map (line 308) without synchronization. MutableMap is not thread-safe for concurrent writes, which can lead to lost updates or map corruption.
🔒 Recommended fix using synchronized access
fun loadFeeAssetMetadata() {
val pending = _state.value.feeAssets.filter { it.address !in resolvedFeeAssets }
if (pending.isEmpty()) return
viewModelScope.launch {
val manager = kit.jettons()
val network = wallet.network()
pending.forEach { asset ->
launch {
runCatching {
val info = manager.jettonInfo(TONUserFriendlyAddress(asset.address), network)
?: return@launch
val symbol = info.symbol.ifEmpty { info.name }.ifEmpty { asset.symbol }
val resolved = asset.copy(
symbol = symbol,
decimals = info.decimals ?: asset.decimals,
imageSource = info.image ?: asset.imageSource,
)
- resolvedFeeAssets[resolved.address] = resolved
+ synchronized(resolvedFeeAssets) {
+ resolvedFeeAssets[resolved.address] = resolved
+ }
_state.update { state ->
state.copy(
feeAssets = state.feeAssets.map { if (it.address == resolved.address) resolved else it },
selectedFeeAsset = state.selectedFeeAsset
?.takeIf { it.address == resolved.address }
?.let { resolved }
?: state.selectedFeeAsset,
)
}
}.onFailure { Log.e(TAG, "Failed to load jetton info for ${asset.address}", it) }
}
}
}
}Alternatively, replace mutableMapOf() at line 99 with ConcurrentHashMap() (requires import java.util.concurrent.ConcurrentHashMap).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| /** | |
| * Resolve fee-asset jetton metadata (ticker / decimals / icon) via [ITONWalletKit.jettons]. | |
| */ | |
| fun loadFeeAssetMetadata() { | |
| val pending = _state.value.feeAssets.filter { it.address !in resolvedFeeAssets } | |
| if (pending.isEmpty()) return | |
| viewModelScope.launch { | |
| val manager = kit.jettons() | |
| val network = wallet.network() | |
| pending.forEach { asset -> | |
| launch { | |
| runCatching { | |
| val info = manager.jettonInfo(TONUserFriendlyAddress(asset.address), network) | |
| ?: return@launch | |
| val symbol = info.symbol.ifEmpty { info.name }.ifEmpty { asset.symbol } | |
| val resolved = asset.copy( | |
| symbol = symbol, | |
| decimals = info.decimals ?: asset.decimals, | |
| imageSource = info.image ?: asset.imageSource, | |
| ) | |
| resolvedFeeAssets[resolved.address] = resolved | |
| _state.update { state -> | |
| state.copy( | |
| feeAssets = state.feeAssets.map { if (it.address == resolved.address) resolved else it }, | |
| selectedFeeAsset = state.selectedFeeAsset | |
| ?.takeIf { it.address == resolved.address } | |
| ?.let { resolved } | |
| ?: state.selectedFeeAsset, | |
| ) | |
| } | |
| }.onFailure { Log.e(TAG, "Failed to load jetton info for ${asset.address}", it) } | |
| } | |
| } | |
| } | |
| } | |
| /** | |
| * Resolve fee-asset jetton metadata (ticker / decimals / icon) via [ITONWalletKit.jettons]. | |
| */ | |
| fun loadFeeAssetMetadata() { | |
| val pending = _state.value.feeAssets.filter { it.address !in resolvedFeeAssets } | |
| if (pending.isEmpty()) return | |
| viewModelScope.launch { | |
| val manager = kit.jettons() | |
| val network = wallet.network() | |
| pending.forEach { asset -> | |
| launch { | |
| runCatching { | |
| val info = manager.jettonInfo(TONUserFriendlyAddress(asset.address), network) | |
| ?: return@launch | |
| val symbol = info.symbol.ifEmpty { info.name }.ifEmpty { asset.symbol } | |
| val resolved = asset.copy( | |
| symbol = symbol, | |
| decimals = info.decimals ?: asset.decimals, | |
| imageSource = info.image ?: asset.imageSource, | |
| ) | |
| synchronized(resolvedFeeAssets) { | |
| resolvedFeeAssets[resolved.address] = resolved | |
| } | |
| _state.update { state -> | |
| state.copy( | |
| feeAssets = state.feeAssets.map { if (it.address == resolved.address) resolved else it }, | |
| selectedFeeAsset = state.selectedFeeAsset | |
| ?.takeIf { it.address == resolved.address } | |
| ?.let { resolved } | |
| ?: state.selectedFeeAsset, | |
| ) | |
| } | |
| }.onFailure { Log.e(TAG, "Failed to load jetton info for ${asset.address}", it) } | |
| } | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@AndroidDemo/app/src/main/java/io/ton/walletkit/demo/presentation/viewmodel/SendTokensViewModel.kt`
around lines 288 - 323, The loadFeeAssetMetadata function launches multiple
concurrent coroutines that each write to the shared resolvedFeeAssets mutable
map without synchronization, creating a race condition. Replace the
initialization of the resolvedFeeAssets field (currently mutableMapOf()) with
ConcurrentHashMap() to provide thread-safe concurrent access, and add the import
statement for java.util.concurrent.ConcurrentHashMap at the top of the file.
Comment on lines
+361
to
+399
| /** | ||
| * Compute "max" for a gasless transfer whose fee is paid in the jetton being sent: probe the relay | ||
| * for the (amount- and recipient-independent) fee, then set the amount to balance − fee. Probing | ||
| * with an affordable amount avoids the HTTP 400 the relay returns when transfer + fee > balance. | ||
| */ | ||
| private fun computeGaslessMax(token: SendableToken, feeAsset: FeeAsset, relay: TONUserFriendlyAddress) { | ||
| quoteJob?.cancel() | ||
| quoteJob = viewModelScope.launch { | ||
| _state.update { it.copy(isQuoting = true, gaslessError = null) } | ||
| runCatching { | ||
| val gasless = ensureGasless() | ||
| val balanceRaw = toRaw(token.displayBalance, token.decimals).toBigInteger() | ||
| // Fee is gas-based — independent of amount and recipient — so probe with half the balance | ||
| // (always affordable) to our own address when no recipient has been entered yet. | ||
| val recipient = _state.value.recipient.trim().takeIf { isValidRecipient(it) } | ||
| ?: wallet.address().value | ||
| val probeAmount = (balanceRaw / BigInteger.valueOf(2)).max(BigInteger.ONE) | ||
| val feeRaw = quote(gasless, token, feeAsset, recipient, probeAmount.toString(), relay) | ||
| .fee.toBigIntegerOrNull() ?: BigInteger.ZERO | ||
| val maxRaw = balanceRaw - feeRaw | ||
| if (maxRaw <= BigInteger.ZERO) { | ||
| _state.update { | ||
| it.copy( | ||
| isQuoting = false, | ||
| gaslessFeeText = null, | ||
| gaslessError = "Balance too low to cover the gasless fee", | ||
| ) | ||
| } | ||
| return@launch | ||
| } | ||
| // Set the amount only; the fee is shown by scheduleQuote() once a recipient is entered. | ||
| _state.update { it.copy(amount = formatRaw(maxRaw.toString(), token.decimals), isQuoting = false) } | ||
| scheduleQuote() | ||
| }.onFailure { | ||
| Log.e(TAG, "Failed to compute gasless max", it) | ||
| _state.update { it.copy(isQuoting = false, gaslessError = "Failed to estimate gasless fee") } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Fee parsing failure is silently converted to zero.
At lines 378-379, if quote().fee cannot be parsed to BigInteger, it silently falls back to BigInteger.ZERO, which would incorrectly compute maxRaw = balanceRaw (line 380) and allow sending more than the user actually has (since the fee would not be subtracted). While this is unlikely if the gasless provider returns well-formed fee strings, it degrades correctness guarantees.
🛡️ Proposed fix to handle fee parsing errors explicitly
- val feeRaw = quote(gasless, token, feeAsset, recipient, probeAmount.toString(), relay)
- .fee.toBigIntegerOrNull() ?: BigInteger.ZERO
+ val quoteResult = quote(gasless, token, feeAsset, recipient, probeAmount.toString(), relay)
+ val feeRaw = quoteResult.fee.toBigIntegerOrNull()
+ ?: throw IllegalStateException("Invalid fee from gasless provider: ${quoteResult.fee}")
val maxRaw = balanceRaw - feeRawThis ensures that malformed fee responses fail explicitly rather than silently computing an incorrect maximum.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| /** | |
| * Compute "max" for a gasless transfer whose fee is paid in the jetton being sent: probe the relay | |
| * for the (amount- and recipient-independent) fee, then set the amount to balance − fee. Probing | |
| * with an affordable amount avoids the HTTP 400 the relay returns when transfer + fee > balance. | |
| */ | |
| private fun computeGaslessMax(token: SendableToken, feeAsset: FeeAsset, relay: TONUserFriendlyAddress) { | |
| quoteJob?.cancel() | |
| quoteJob = viewModelScope.launch { | |
| _state.update { it.copy(isQuoting = true, gaslessError = null) } | |
| runCatching { | |
| val gasless = ensureGasless() | |
| val balanceRaw = toRaw(token.displayBalance, token.decimals).toBigInteger() | |
| // Fee is gas-based — independent of amount and recipient — so probe with half the balance | |
| // (always affordable) to our own address when no recipient has been entered yet. | |
| val recipient = _state.value.recipient.trim().takeIf { isValidRecipient(it) } | |
| ?: wallet.address().value | |
| val probeAmount = (balanceRaw / BigInteger.valueOf(2)).max(BigInteger.ONE) | |
| val feeRaw = quote(gasless, token, feeAsset, recipient, probeAmount.toString(), relay) | |
| .fee.toBigIntegerOrNull() ?: BigInteger.ZERO | |
| val maxRaw = balanceRaw - feeRaw | |
| if (maxRaw <= BigInteger.ZERO) { | |
| _state.update { | |
| it.copy( | |
| isQuoting = false, | |
| gaslessFeeText = null, | |
| gaslessError = "Balance too low to cover the gasless fee", | |
| ) | |
| } | |
| return@launch | |
| } | |
| // Set the amount only; the fee is shown by scheduleQuote() once a recipient is entered. | |
| _state.update { it.copy(amount = formatRaw(maxRaw.toString(), token.decimals), isQuoting = false) } | |
| scheduleQuote() | |
| }.onFailure { | |
| Log.e(TAG, "Failed to compute gasless max", it) | |
| _state.update { it.copy(isQuoting = false, gaslessError = "Failed to estimate gasless fee") } | |
| } | |
| } | |
| } | |
| /** | |
| * Compute "max" for a gasless transfer whose fee is paid in the jetton being sent: probe the relay | |
| * for the (amount- and recipient-independent) fee, then set the amount to balance − fee. Probing | |
| * with an affordable amount avoids the HTTP 400 the relay returns when transfer + fee > balance. | |
| */ | |
| private fun computeGaslessMax(token: SendableToken, feeAsset: FeeAsset, relay: TONUserFriendlyAddress) { | |
| quoteJob?.cancel() | |
| quoteJob = viewModelScope.launch { | |
| _state.update { it.copy(isQuoting = true, gaslessError = null) } | |
| runCatching { | |
| val gasless = ensureGasless() | |
| val balanceRaw = toRaw(token.displayBalance, token.decimals).toBigInteger() | |
| // Fee is gas-based — independent of amount and recipient — so probe with half the balance | |
| // (always affordable) to our own address when no recipient has been entered yet. | |
| val recipient = _state.value.recipient.trim().takeIf { isValidRecipient(it) } | |
| ?: wallet.address().value | |
| val probeAmount = (balanceRaw / BigInteger.valueOf(2)).max(BigInteger.ONE) | |
| val quoteResult = quote(gasless, token, feeAsset, recipient, probeAmount.toString(), relay) | |
| val feeRaw = quoteResult.fee.toBigIntegerOrNull() | |
| ?: throw IllegalStateException("Invalid fee from gasless provider: ${quoteResult.fee}") | |
| val maxRaw = balanceRaw - feeRaw | |
| if (maxRaw <= BigInteger.ZERO) { | |
| _state.update { | |
| it.copy( | |
| isQuoting = false, | |
| gaslessFeeText = null, | |
| gaslessError = "Balance too low to cover the gasless fee", | |
| ) | |
| } | |
| return@launch | |
| } | |
| // Set the amount only; the fee is shown by scheduleQuote() once a recipient is entered. | |
| _state.update { it.copy(amount = formatRaw(maxRaw.toString(), token.decimals), isQuoting = false) } | |
| scheduleQuote() | |
| }.onFailure { | |
| Log.e(TAG, "Failed to compute gasless max", it) | |
| _state.update { it.copy(isQuoting = false, gaslessError = "Failed to estimate gasless fee") } | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@AndroidDemo/app/src/main/java/io/ton/walletkit/demo/presentation/viewmodel/SendTokensViewModel.kt`
around lines 361 - 399, In the computeGaslessMax function, the fee parsing from
quote().fee is using toBigIntegerOrNull() with a silent fallback to
BigInteger.ZERO via the elvis operator, which masks parsing failures and causes
incorrect maxRaw calculation when the fee cannot be parsed. Replace the silent
fallback with explicit error handling that throws an exception or sets an error
state when the fee string cannot be parsed to BigInteger, ensuring that
malformed fee responses from the relay fail explicitly rather than allowing the
user to incorrectly send their entire balance without accounting for the fee.
Comment on lines
+100
to
+101
| @Serializable | ||
| internal data class ValidateJettonAddressResponse(val valid: Boolean = false) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether bridge payload for validateJettonAddress always includes `valid`
# and confirm decode path used by callTyped.
set -euo pipefail
echo "== Locate validateJettonAddress implementation in JS bridge =="
rg -n -C3 '\bvalidateJettonAddress\b' TONWalletKit-Android/impl/src/main/assets/walletkit/walletkit-android-bridge.mjs
echo
echo "== Locate callTyped implementation and decode behavior =="
fd -i 'BridgeRpcClient.kt|RpcClient.kt|.*Operations.kt' TONWalletKit-Android/impl/src/main/java | xargs rg -n -C3 '\bcallTyped\b|\bdecodeFromString\b|\bdecodeFromJsonElement\b'
echo
echo "== Locate all usages of ValidateJettonAddressResponse =="
rg -n -C2 '\bValidateJettonAddressResponse\b'Repository: ton-org/kit-android
Length of output: 50375
🏁 Script executed:
# Check the complete ValidateJettonAddressResponse definition and similar response classes
rg -A2 -B2 'internal data class.*Response.*valid|internal data class.*Response.*Boolean' TONWalletKit-Android/impl/src/main/java/io/ton/walletkit/engine/operations/responses/ | head -100
# Also check how kotlinx.serialization handles defaults
rg -C5 '`@Serializable`' TONWalletKit-Android/impl/src/main/java/io/ton/walletkit/engine/operations/responses/BridgeResponses.kt | grep -A3 -B3 'Boolean\|valid'Repository: ton-org/kit-android
Length of output: 1411
🏁 Script executed:
# Check the JS bridge to see all code paths for validateJettonAddress and whether valid could be undefined
cat TONWalletKit-Android/impl/src/main/assets/walletkit/walletkit-android-bridge.mjs | sed -n '40870,40875p'
# Check if there are other response models that intentionally DON'T have defaults
rg 'data class.*Response\(.*Boolean\)' TONWalletKit-Android/impl/src/main/java/io/ton/walletkit/engine/operations/responses/ | head -20Repository: ton-org/kit-android
Length of output: 288
🏁 Script executed:
# Check the underlying jettons.validateJettonAddress implementation
rg -B5 -A10 'validateJettonAddress.*try\|validateJettonAddress\(.*{' TONWalletKit-Android/impl/src/main/assets/walletkit/walletkit-android-bridge.mjs | head -30
# Check if there are response classes WITHOUT defaults to see the pattern
rg 'internal data class.*Response\(' TONWalletKit-Android/impl/src/main/java/io/ton/walletkit/engine/operations/responses/BridgeResponses.kt | grep -v '= ' | head -20
# Check callers of validateJettonAddress to see how errors are handled
rg -B3 -A3 'validateJettonAddress\(' TONWalletKit-Android/impl/src/main/java | grep -v '.mjs'Repository: ton-org/kit-android
Length of output: 4257
🏁 Script executed:
# Look at response classes without defaults to understand the pattern
rg -A3 'internal data class.*Response\(' TONWalletKit-Android/impl/src/main/java/io/ton/walletkit/engine/operations/responses/BridgeResponses.kt | grep -A3 'KeyPairResponse\|ProviderIdResponse\|SignerInfoResponse'
# Search for the jettons implementation in the bridge to see if validateJettonAddress can fail or return undefined
rg -B10 'validateJettonAddress\(' TONWalletKit-Android/impl/src/main/assets/walletkit/walletkit-android-bridge.mjs | grep -A10 'validateJettonAddress' | head -20Repository: ton-org/kit-android
Length of output: 1298
🏁 Script executed:
# Check the underlying jettons implementation more carefully
rg -B20 'validateJettonAddress\(' TONWalletKit-Android/impl/src/main/assets/walletkit/walletkit-android-bridge.mjs | tail -30
# Check if there's error handling around getKit() that could fail
rg -B5 -A5 'getKit\(\)' TONWalletKit-Android/impl/src/main/assets/walletkit/walletkit-android-bridge.mjs | head -40
# Look at the actual ton/jettons validateJettonAddress method from the bridge to understand what it returns
rg -C3 'isTonAddress\|Address.parse' TONWalletKit-Android/impl/src/main/assets/walletkit/walletkit-android-bridge.mjs | head -30Repository: ton-org/kit-android
Length of output: 2429
🏁 Script executed:
# Check if HasProviderResponse has the same pattern and if it should also be fixed
rg -B5 -A10 'internal data class HasProviderResponse' TONWalletKit-Android/impl/src/main/java/io/ton/walletkit/engine/operations/responses/BridgeResponses.kt
# Check callers of hasSwapProvider to understand error handling
rg 'hasSwapProvider' TONWalletKit-Android/impl/src/main/java/io/ton/walletkit/engine/operations/ -A3 -B1Repository: ton-org/kit-android
Length of output: 1186
Remove the = false default from ValidateJettonAddressResponse.valid.
The JS bridge implementation always includes the valid field in its response object. A default value masks protocol/schema regressions as legitimate "invalid address" results when the field is unexpectedly omitted, conflating two semantically distinct failure modes. Required response fields should not have defaults.
Proposed fix
`@Serializable`
-internal data class ValidateJettonAddressResponse(val valid: Boolean = false)
+internal data class ValidateJettonAddressResponse(val valid: Boolean)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| @Serializable | |
| internal data class ValidateJettonAddressResponse(val valid: Boolean = false) | |
| `@Serializable` | |
| internal data class ValidateJettonAddressResponse(val valid: Boolean) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@TONWalletKit-Android/impl/src/main/java/io/ton/walletkit/engine/operations/responses/BridgeResponses.kt`
around lines 100 - 101, The ValidateJettonAddressResponse data class has a
default value of false assigned to the valid field, which can mask protocol
failures when the field is unexpectedly omitted from the response. Remove the
default value assignment from the valid field in ValidateJettonAddressResponse
to make it a required field, ensuring that deserialization fails clearly if the
JS bridge omits the valid field rather than silently defaulting to false.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…on and address validation
Summary by CodeRabbit
New Features
Chores