Jettons swap #17
Jettons swap #17
Conversation
Ease review and code comprehension. Prerequisite to jetton wallet address check.
move code from transaction_hints.c to jetton.c and add helper function. this will be requires for jetton swap address check.
and use the new jetton helper functions.
…ansfer. For a Jetton transfer, transaction message is sent to owner jetton wallet address, the recipient address is serialized in transaction hints.
Fix some type annotation non py39 compliant (this is the python version packaged in build/test docker images. Fix pathlib Path concatenation, do not use hardcoded path separator but the `/` operator or Path initialization (w/ accepts *args as path fragment to assemble).
fvalette-ledger
force-pushed
the
jettons-swap
branch
from
29fa7c1 to
7339955
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #17 +/- ##
========================================
Coverage 68.72% 68.72%
========================================
Files 8 8
Lines 243 243
========================================
Hits 167 167
Misses 76 76
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Some fixes are weird though, those are not compliant w/ configuration (...)
those changes are not related to the current feature.
those changes are not related to the current feature.
Disposable fix, this will be remove by Pull Request #16
This case shall not occurs as the transaction type is enforced and, by construction, hints must be valid (also checked). This is unlikely to got a null pointer here but static analyser can't figure it out.
|
Remember to bump Makefile version |
| /* if ticker different from `TON`, address to check is a jetton wallet address */ | ||
| if (strncmp(ticker, "TON", sizeof("TON")) != 0) { | ||
| if (!swap_get_jetton_wallet_address(ticker, hash)) { | ||
| return; |
There was a problem hiding this comment.
Hésite pas à rajouter plein de logs pour ce genre de cas de figure, si un jour on a un pb en prod ça sera bien plus simple à investiguer
| return; | ||
| } | ||
|
|
||
| if (!swap_get_addr_hash_from_path(params, hash, HASH_LEN)) { | ||
| return; |
| } | ||
| #endif | ||
|
|
||
| if (!swap_check_address_consistency(params, hash)) { | ||
| return; |
| io_send_sw(SW_SWAP_FAILURE); | ||
| // unreachable | ||
| os_sched_exit(0); | ||
| } else { | ||
| PRINTF("Valid operation %d\n", G_context.tx_info.transaction.hints_type); | ||
| } |
There was a problem hiding this comment.
I'm not certain but I think we should be more strict in this check
If we are swapping jettons (currency is not TON) == we require a TRANSACTION_TRANSFER_JETTON
Otherwise we forbid it
Already defined in secure SDK.
|
|
||
| if (params->address_parameters == NULL) { | ||
| PRINTF("derivation path expected\n"); | ||
| return; | ||
| goto out; |
There was a problem hiding this comment.
might as well return false, more explicit and same behavior
| if (get_public_key_helper(0, &cdata, &bip32_path_len, bip32_path, &pk_info) != 0) { | ||
| PRINTF("Failed to read public key\n"); | ||
| return; | ||
| goto out; |
| ret = true; | ||
| out: | ||
| return ret; |
There was a problem hiding this comment.
I can understand using gotos when there is a context to clean but I think having them here does not provide any benefit
There was a problem hiding this comment.
The benefit is a cleaner stack frame and single exit point per function.
this already exists.
Checklist
develop