Add tokenId denom conversion utils #688
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| // Address information: | ||
| payoutAddress: asString, | ||
| payoutCurrencyCode: asString, | ||
| payoutTokenId: asEdgeTokenId, |
There was a problem hiding this comment.
Bug: Required field breaks loading old transaction files
The payoutTokenId field in asEdgeTxSwap is required but old transaction files saved to disk won't have this field. When loading existing transactions via transactionFile.load, the cleaner will fail validation. The field needs to be wrapped with asOptional to handle backward compatibility with existing data, since upgradeSwapData only runs during makeSpend and not when loading persisted transactions.
There was a problem hiding this comment.
This, exactly. We cannot add new mandatory fields to old data. If we truly wanted to go down this path, we would first have to add the tokenId as an optional field, and then incorporate a migration path for old data.
However, since this is a strict save-and-return workflow, I would propose not to add a migration path, but to simply add the payoutTokenId: asOptional(asEdgeTokenId), and a payoutTokenId?: EdgeTokenId to the types. That way we don't mess with old data, and the GUI is the one who has to deal with any backwards-compatibility issues. This backwards-compatibility is not terribly complex, since the GUI pretty much just shows this value as-is on the tx details scene.
| // Address information: | ||
| payoutAddress: asString, | ||
| payoutCurrencyCode: asString, | ||
| payoutTokenId: asEdgeTokenId, |
There was a problem hiding this comment.
This, exactly. We cannot add new mandatory fields to old data. If we truly wanted to go down this path, we would first have to add the tokenId as an optional field, and then incorporate a migration path for old data.
However, since this is a strict save-and-return workflow, I would propose not to add a migration path, but to simply add the payoutTokenId: asOptional(asEdgeTokenId), and a payoutTokenId?: EdgeTokenId to the types. That way we don't mess with old data, and the GUI is the one who has to deal with any backwards-compatibility issues. This backwards-compatibility is not terribly complex, since the GUI pretty much just shows this value as-is on the tx details scene.
| export function getTokenMultiplier( | ||
| currencyInfo: EdgeCurrencyInfo, | ||
| allTokens: EdgeTokenMap, | ||
| tokenId: EdgeTokenId | ||
| ): string { | ||
| if (tokenId == null) { | ||
| for (const denomination of currencyInfo.denominations) { | ||
| if (denomination.name === currencyInfo.currencyCode) { | ||
| return denomination.multiplier | ||
| } | ||
| } | ||
| } else { | ||
| const token: EdgeToken | undefined = allTokens[tokenId] | ||
| if (token != null) { | ||
| for (const denomination of token.denominations) { | ||
| if (denomination.name === token.currencyCode) { | ||
| return denomination.multiplier | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return '1' | ||
| } | ||
|
|
There was a problem hiding this comment.
The core should not provide utility functions like this, since everything goes over an expensive bridge. The denominationToNative and nativeToDenomination functions were historical accidents, and deprecation message says to "Use the information in EdgeCurrencyInfo / EdgeToken". We want to delete these guys, not replace them.
These types of utilities should move into edge-currency-accountbased and edge-react-gui in some sort of /utils file, since then they can be fast & synchronous.
CHANGELOG
Does this branch warrant an entry to the CHANGELOG?
Dependencies
noneDescription
noneNote
Adds tokenId-based denomination conversion methods to wallets and extends swap data with payoutTokenId, including upgrade logic.
convertDenominatedToNative(tokenId)andconvertNativeToDenominated(tokenId)onEdgeCurrencyWallet.makeSpendnow cleansswapDataand upgrades it when present.getTokenMultiplier; markgetCurrencyMultiplieras deprecated in favor of it.payoutTokenIdtoEdgeTxSwapand corresponding cleaner.EdgeCurrencyWalletinterface with the new conversion methods.upgradeSwapData(tx)to backfillswapData.payoutTokenIdfromsavedActionwhen missing.payoutTokenIdand exercise new behavior.payoutTokenIdaddition.Written by Cursor Bugbot for commit ac53f48. This will update automatically on new commits. Configure here.