From d31b490c811988dc96a60a14e740cf34065a7e77 Mon Sep 17 00:00:00 2001 From: gobicycle Date: Tue, 27 Jun 2023 22:14:26 +0300 Subject: [PATCH 1/9] todo updates --- todo_list.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/todo_list.md b/todo_list.md index 86da434..1e97e10 100644 --- a/todo_list.md +++ b/todo_list.md @@ -63,3 +63,6 @@ - [ ] Not process removed Jettons - [ ] Separate .env files for services - [ ] Automatic migrations +- [ ] SDK +- [ ] Cold wallet withdrawal fix +- [ ] migration from blueprint to openapi \ No newline at end of file From 4818ac290997be6772f57ec491c402f5bc38ec6a Mon Sep 17 00:00:00 2001 From: gobicycle Date: Sun, 2 Jul 2023 14:01:34 +0300 Subject: [PATCH 2/9] todo updates --- todo_list.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/todo_list.md b/todo_list.md index 1e97e10..7dc8edd 100644 --- a/todo_list.md +++ b/todo_list.md @@ -48,9 +48,12 @@ - [x] Add filling deposit with bounce to test plan - [x] Update to tonutils-go 1.6.2 - [x] Process masterchain addresses for external incomes +- [ ] Cold wallet withdrawal fix +- [ ] Add user id to notifications +- [ ] Add transaction hash to notifications +- [ ] Support DNS names in recipient address - [ ] Jetton threat model - [ ] TNX compatibility test -- [ ] Support DNS names in recipient address - [ ] Installation video manual - [ ] Use stable branch for emulator - [ ] Download blockchain config at start @@ -64,5 +67,4 @@ - [ ] Separate .env files for services - [ ] Automatic migrations - [ ] SDK -- [ ] Cold wallet withdrawal fix -- [ ] migration from blueprint to openapi \ No newline at end of file +- [ ] migration from blueprint to openapi From 514bc91642cd1324816cb1f0e39ccc98a4463f30 Mon Sep 17 00:00:00 2001 From: gobicycle Date: Sun, 2 Jul 2023 15:40:52 +0300 Subject: [PATCH 3/9] cold wallet address format validation --- README.md | 2 +- cmd/processor/main.go | 2 +- core/wallets.go | 12 +++++ manual_testing_plan.md | 107 +++++++++++++++++++++++------------------ threat_model.md | 11 ++++- todo_list.md | 3 +- 6 files changed, 85 insertions(+), 52 deletions(-) diff --git a/README.md b/README.md index cba7f0c..097e8f4 100644 --- a/README.md +++ b/README.md @@ -81,7 +81,7 @@ For more information on Jettons compatibility, see [Jettons compatibility](/jett | `IS_TESTNET` | `true` if service works in TESTNET, `false` - for MAINNET. Default: `true`. | | `JETTONS` | list of Jettons, processed by service in format:
`JETTON_SYMBOL_1:MASTER_CONTRACT_ADDR_1:hot_wallet_max_balance:min_withdrawal_amount, JETTON_SYMBOL_2:MASTER_CONTRACT_ADDR_2:hot_wallet_max_balance:min_withdrawal_amount`,
example: `TGR:kQCKt2WPGX-fh0cIAz38Ljd_OKQjoZE_cqk7QrYGsNP6wfP0:1000000:100000` | | `TON_CUTOFFS` | cutoffs in nanoTONs in format:
`hot_wallet_min_balance:hot_wallet_max_balance:min_withdrawal_amount`,
example `1000000000:100000000000:1000000000` | -| `COLD_WALLET` | cold-wallet address, example `kQCdyiS-fIV9UVfI9Phswo4l2MA-hm8YseH3XZ_YiH9Y1ufw` | +| `COLD_WALLET` | cold-wallet address, example `kQCdyiS-fIV9UVfI9Phswo4l2MA-hm8YseH3XZ_YiH9Y1ufw`. If cold wallet is not active - use non-bounceable address (use https://ton.org/address for convert) | | `DEPOSIT_SIDE_BALANCE` | `true` - service calculates total income for user by deposit incoming, `false` - by hot wallet incoming. Default: `true`. | | `QUEUE_ENABLED` | `true` - service sends incoming notifications to queue, `false` - sending disabled. Default: `false`. | | `QUEUE_URI` | URI for queue client connection, example `amqp://guest:guest@payment_rabbitmq:5672/` | diff --git a/cmd/processor/main.go b/cmd/processor/main.go index 27c880e..265d967 100644 --- a/cmd/processor/main.go +++ b/cmd/processor/main.go @@ -54,7 +54,7 @@ func main() { wallets, err := core.InitWallets(ctx, dbClient, bcClient, config.Config.Seed, config.Config.Jettons) if err != nil { - log.Fatalf("Hot wallets initialization error: %v", err) + log.Fatalf("Wallets initialization error: %v", err) } var notificators []core.Notificator diff --git a/core/wallets.go b/core/wallets.go index 2d97825..fb010ad 100644 --- a/core/wallets.go +++ b/core/wallets.go @@ -33,6 +33,18 @@ func InitWallets( seed string, jettons map[string]config.Jetton, ) (Wallets, error) { + + if config.Config.ColdWallet != nil && config.Config.ColdWallet.IsBounceable() { + _, status, err := bc.GetAccountCurrentState(ctx, config.Config.ColdWallet) + if err != nil { + return Wallets{}, err + } + log.Infof("Cold wallet status: %s", status) + if status != tlb.AccountStatusActive { + return Wallets{}, fmt.Errorf("cold wallet address must be non-bounceable for not active wallet") + } + } + tonHotWallet, shard, subwalletId, err := initTonHotWallet(ctx, db, bc, seed) if err != nil { return Wallets{}, err diff --git a/manual_testing_plan.md b/manual_testing_plan.md index 3b95a7f..768af87 100644 --- a/manual_testing_plan.md +++ b/manual_testing_plan.md @@ -1,4 +1,4 @@ -## Manual testing plan for v0.2.0 +## Manual testing plan for v0.4.0 Template: -[x] Checked - TEST : test description @@ -7,59 +7,64 @@ Template: ### Initialization -1. -[x] Checked +1. -[ ] Checked - TEST : Run with not deployed (and zero balance) hot wallet (new seed phrase) - RESULT : There must be an insufficient balance error - COMMENT : -2. -[x] Checked +2. -[ ] Checked - TEST : Run with uninit hot wallet with balance > minimum balance - RESULT : Hot wallet must be initialized at first start of service - COMMENT : -3. -[x] Checked +3. -[ ] Checked - TEST : Run with new seed phrase when hot wallet already exist in DB - RESULT : There must be an incorrect seed phrase error - COMMENT : -4. -[x] Checked +4. -[ ] Checked - TEST : Run service with empty DB and stop after few minutes. Check time of first and last block in `block_data` table - RESULT : Time `saved_at` and `gen_utime` must correlate with system time - COMMENT : -5. -[x] Checked +5. -[ ] Checked - TEST : Run with nonexist hot jetton wallet and receive external jetton transfer at jetton deposit (> minimal withdrawal amount) - RESULT : Jetton hot wallet must be initialized by Jetton withdrawal from deposit, if jetton deposit successfully initialized (it depends on transfer sender) - COMMENT : -6. -[x] Checked +6. -[ ] Checked - TEST : Run with testnet cold wallet address at mainnet (`IS_TESTNET=false`) - RESULT : There must be "Can not use testnet cold wallet address for mainnet" error - COMMENT : -7. -[x] Checked +7. -[ ] Checked - TEST : Run service with empty `JETTONS` env variable - RESULT : Service must start and process TONs - COMMENT : -8. -[x] Checked +8. -[ ] Checked - TEST : Run service with `JETTONS` env variable with different currencies and same master contract address. Like `TGR:ABC...,FNZ:ABC...`. - RESULT : Service must stop. Must be address duplication error message in audit log. - COMMENT : -9. -[x] Checked +9. -[ ] Checked - TEST : Run service with one `JETTONS` env variable, then rename currency for one of Jetton and restart. Like `TGR:ABC...,FNZ:CDE...` -> `SCALE:ABC...,FNZ:CDE...`. - RESULT : Service must stop. Must be address duplication error message in audit log. - COMMENT : +10. -[x] Checked +- TEST : Start service with uninitialized cold wallet and bounceable address for cold wallet. +- RESULT : Service must stop. Must be invalid address format error message in log. +- COMMENT : + ### API -1. -[x] Checked +1. -[ ] Checked - TEST : Use `/v1/address/new` method (few for TONs and few for Jettons for different users). Check new addresses in DB - RESULT : You must receive different addresses in user-friendly format with `bounce = false` flag and @@ -67,23 +72,23 @@ Template: owner address. - COMMENT : -2. -[x] Checked +2. -[ ] Checked - TEST : Use `/v1/address/all{?user_id}` method and compare with addresses created at 1. And check it by DB - RESULT : All addresses must be received and equal to those created earlier - COMMENT : -3. -[x] Checked +3. -[ ] Checked - TEST : Check `/v1/income{?user_id}` for new empty deposits - RESULT : Income must be zero. The addresses must match the addresses obtained by method `/v1/address/all{?user_id}`. - COMMENT : -4. -[x] Checked +4. -[ ] Checked - TEST : Make some payments at deposits and check it by `/v1/income{?user_id}` method with different `DEPOSIT_SIDE_BALANCE` env var - RESULT : Income must correlate with payments sum - COMMENT : -5. -[x] Checked +5. -[ ] Checked - TEST : Make some withdrawals by `/v1/withdrawal/send` method for TONs and Jettons with amount > hot wallet balance and check it by `/v1/withdrawal/status{?id}` few times. Check status of withdrawals by transaction explorer (e.g. https://testnet.tonapi.io/ or https://tonapi.io/). Check withdrawal in DB. @@ -91,7 +96,7 @@ Template: There is no any correlated messages in `external withdrawals` table. - COMMENT : -6. -[x] Checked +6. -[ ] Checked - TEST : Make some withdrawals by `/v1/withdrawal/send` method for TONs and check it by `/v1/withdrawal/status{?id}` few times and try to catch all statuses: `pending`, `processing`, `processed`. Check status of withdrawals by transaction explorer (e.g. https://testnet.tonapi.io/ or https://tonapi.io/). Check withdrawal in DB. @@ -100,7 +105,7 @@ Template: as final state. - COMMENT : -7. -[x] Checked +7. -[ ] Checked - TEST : Make some withdrawals by `/v1/withdrawal/send` method for Jettons with not deployed Jetton hot wallets and withdrawals by `/v1/withdrawal/status{?id}` few times. Check status of withdrawals by transaction explorer (e.g. https://testnet.tonapi.io/ or https://tonapi.io/). Check withdrawal in DB. @@ -108,7 +113,7 @@ Template: There is no any correlated messages in `external withdrawals` table. - COMMENT : -8. -[x] Checked +8. -[ ] Checked - TEST : Make some withdrawals by `/v1/withdrawal/send` method for Jettons with deployed Jetton hot wallets and check it by `/v1/withdrawal/status{?id}` few times and try to catch all statuses: `pending`, `processing`, `processed`. Check status of withdrawals by transaction explorer @@ -118,52 +123,52 @@ Template: as final state. - COMMENT : -9. -[x] Checked +9. -[ ] Checked - TEST : Start the service after some downtime. Check sync status by `/v1/system/sync` few times. - RESULT : Start status should be `"is_synced": false` then become `"is_synced": true` - COMMENT : -10. -[x] Checked +10. -[ ] Checked - TEST : Start service with `IS_TESTNET=true` env var. Make some withdrawals by `/v1/withdrawal/send` method to TESTNET and MAINNET user-friendly form addresses. - RESULT : All withdrawals must be accepted - COMMENT : -11. -[x] Checked +11. -[ ] Checked - TEST : Start service with `IS_TESTNET=false` env var. Make some withdrawals by `/v1/withdrawal/send` method to TESTNET user-friendly form addresses. - RESULT : All withdrawals must be rejected - COMMENT : -12. -[x] Checked +12. -[ ] Checked - TEST : Make some withdrawals by `/v1/withdrawal/send` method to service internal addresses (hot wallet, jetton hot wallet, owner, ton deposit, jetton deposit). - RESULT : All withdrawals must be rejected - COMMENT : -13. -[x] Checked +13. -[ ] Checked - TEST : Try all methods with auth using wrong token - RESULT : All requests must be rejected - COMMENT : -14. -[x] Checked +14. -[ ] Checked - TEST : Make TON and Jetton withdrawal to -1 workchain - RESULT : Withdrawals must be rejected - COMMENT : -15. -[x] Checked +15. -[ ] Checked - TEST : Make withdrawals by `/v1/withdrawal/service/ton` and `/v1/withdrawal/service/jetton` from unknown address and another network (testnet addr for mainnet address and -1 workchain address) - RESULT : Withdrawals must be rejected - COMMENT : -16. -[x] Checked +16. -[ ] Checked - TEST : Make withdrawal by `/v1/withdrawal/service/ton` and `/v1/withdrawal/service/jetton` from known internal but not Jetton deposit owner and not TON deposit address (hot wallet and Jetton wallet) - RESULT : Withdrawal must be rejected - COMMENT : -17. -[x] Checked +17. -[ ] Checked - TEST : Make TON withdrawal by `/v1/withdrawal/service/ton` from known Jetton deposit owner address. Check for unusual transactions in the database - RESULT : Withdrawal must be accepted. All TONs must be sent from Jetton deposit owner to hot wallet. There is no @@ -171,7 +176,7 @@ Template: must be `processed = true` - COMMENT : -18. -[x] Checked +18. -[ ] Checked - TEST : Make Jetton (not deposit Jetton type) withdrawal by `/v1/withdrawal/service/jetton` from known internal Jetton deposit owner address. Check for unusual transactions in the database - RESULT : Withdrawal must be accepted. Jettons must be sent from Jetton wallet to hot wallet. @@ -180,7 +185,7 @@ Template: - COMMENT : Should be zero forward TON amount in transfer message to prevent invoking notification message and incorrect interpretation hot wallet incoming message -19. -[x] Checked +19. -[ ] Checked - TEST : Make Jetton withdrawal by `/v1/withdrawal/service/jetton` from known internal TON deposit address. Check for unusual transactions in the database - RESULT : Withdrawal must be accepted. First, there must be a TON filling transaction from hot wallet to TON deposit. @@ -192,21 +197,21 @@ Template: TON deposit occurs through the Jetton wallet and is not detected by the block scanner as an internal TON withdrawal. The deposit balance on the hot wallet side is not replenished. -20. -[x] Checked +20. -[ ] Checked - TEST : Make Jetton (for deposit Jetton type) withdrawal by `/v1/withdrawal/service/jetton` from known internal Jetton deposit owner address. - RESULT : In `service_withdrawal_request` DB table should be `processed = true` and `filled=false` with zero balances. Must be warning about rejected withdrawal in audit log - COMMENT : -21. -[x] Checked +21. -[ ] Checked - TEST : Make TON withdrawal by `/v1/withdrawal/service/ton` from Jetton owner address with zero TON balance - RESULT : Withdrawal must be accepted. There should be audit log info about zero balance. There is no any deposit transactions (incomes/withdrawals) in the database and no messages from hot wallet. In `service_withdrawal_request` DB table must be `processed = true` - COMMENT : -22. -[x] Checked +22. -[ ] Checked - TEST : Make Jetton withdrawal by `/v1/withdrawal/service/jetton` from Jetton owner address and from TON deposit address with zero Jetton balance - RESULT : Withdrawal must be accepted. There should be audit log info about zero balance. There is no any deposit @@ -214,7 +219,7 @@ Template: In `service_withdrawal_request` DB table must be `processed = true` - COMMENT : -23. -[x] Checked +23. -[ ] Checked - TEST : Set some Jetton in `JETTONS` env variable. Start service to init jetton hot wallet in DB. Remove Jetton from env variable and restart. Try `/v1/address/new`, `/v1/withdrawal/send` for removed Jetton. - RESULT : Must be currency error for `/v1/address/new`, `/v1/withdrawal/send`. @@ -227,7 +232,7 @@ Template: - RESULT : Removed Jetton should not appear in `/v1/address/all`, `/v1/income`. - COMMENT : Not implemented yet -25. -[x] Checked +25. -[ ] Checked - TEST : Make some payments at deposits and check it by `/v1/history{?user_id,currency,limit,offset}` method with different `DEPOSIT_SIDE_BALANCE` env var - RESULT : Incomes must correlate with payments and DB `external_incomes` table. The history on the deposits side @@ -240,14 +245,14 @@ Template: - RESULT : The sender's address must be displayed correctly in the history. - COMMENT : -27. -[x] Checked +27. -[ ] Checked - TEST : Replenish the TON deposit (when it in nonexist status) with a bounceable message and check it by `/v1/history{?user_id,currency,limit,offset}` method. Also check logs. - RESULT : The bounced payment should not be in the history. There should be no errors in the logs, only a warning about a bounced message. - COMMENT : -28. -[x] Checked +28. -[ ] Checked - TEST : Replenish the Jetton deposit with zero forward amount and check it by `/v1/history{?user_id,currency,limit,offset}` method. - RESULT : The sender's address must be not presented in the history. @@ -255,11 +260,12 @@ Template: ### Internal logic -1. -[x] Checked +1. -[ ] Checked - TEST : Replenish the deposit with TONs and Jettons so that as a result the amount on the hot wallet is greater - than hot_wallet_max_balance. Check withdrawals in DB -- RESULT : You must find new withdrawal in `withdrawal_requests` table with `is_internal=true`. And final status - must correlate with explorer. + than `hot_wallet_max_balance` when cold wallet is not active and cold wallet address in non-bounceable format. + Check withdrawals in DB +- RESULT : You must find new withdrawal in `withdrawal_requests` table with `is_internal=true` and `bounceable=false`. + And final status must correlate with explorer. - COMMENT : 2. -[ ] Checked @@ -268,41 +274,48 @@ Template: - RESULT : There should be no missing blocks in the DB. - COMMENT : +3. -[ ] Checked +- TEST : Replenish the deposit with TONs and Jettons so that as a result the amount on the hot wallet is greater + than `hot_wallet_max_balance`. Check withdrawals in DB +- RESULT : You must find new withdrawal in `withdrawal_requests` table with `is_internal=true`. And final status + must correlate with explorer. Withdrawal amount must correlate with hysteresis formula. +- COMMENT : + ### Deploy -1. -[x] Checked +1. -[ ] Checked - TEST : Build docker images and start `payment-postgres`, `payment-processor` services using README.md instructions Check availability and functionality of service. - RESULT : The API must be accessible and functional - COMMENT : -2. -[x] Checked +2. -[ ] Checked - TEST : Start optional `payment-grafana` service using README.md instructions Check availability and functionality of service. - RESULT : The `payments` Grafana dashboard must be accessible and show DB data - COMMENT : -3. -[x] Checked +3. -[ ] Checked - TEST : Start `payment-processor` with `QUEUE_ENABLED=true` env var and optional `payment-rabbitmq` service using README.md instructions. Make some payments to deposits. Check availability and functionality of service by RabbitMQ dashboard - RESULT : Must be some message activity in RabbitMQ dashboard for exchange - COMMENT : -4. -[x] Checked +4. -[ ] Checked - TEST : Start `payment-test` service using technical_notes.md instructions with `CIRCULATION=false` env variable. Check availability and functionality of service by Grafana dashboard. - RESULT : Grafana must show prometheus metrics from `payment-test` service (deposit and total balances) - COMMENT : -5. -[x] Checked +5. -[ ] Checked - TEST : Start `payment-test` service using technical_notes.md instructions with `CIRCULATION=true` env variable. Check availability and functionality of service by Grafana dashboard. - RESULT : Grafana must show prometheus metrics from `payment-test` service (deposit and total balances) and payment activity - COMMENT : -6. -[x] Checked +6. -[ ] Checked - TEST : Start `payment-processor` with `WEBHOOK_ENDPOINT=http://localhost:3333/webhook` env var. Start test webserver from `cmd/testwebhook/main.go`. Make some payments to deposits. Check payments data at webserver side. Add env variable `WEBHOOK_TOKEN=123` and restart `payment-processor`. Make some payments @@ -311,7 +324,7 @@ Template: `WEBHOOK_TOKEN` is not set. - COMMENT : -7. -[x] Checked +7. -[ ] Checked - TEST : Start `payment-processor` with webhooks. Make Jetton payment to deposits with zero froward amount. Check payments data at webserver side. - RESULT : The sender's address must be not presented. diff --git a/threat_model.md b/threat_model.md index f03b656..5d3c3a3 100644 --- a/threat_model.md +++ b/threat_model.md @@ -122,8 +122,15 @@ If TONs arrive at the wallet address at this time, the message will be applied a - D: warning about this behavior in technical_notes file for method description #### Setting the value to "expired" without taking into account the allowable delay -- P: It is impossible to absolutely precisely synchronize in time with the blockchain, so there is an +- P: it is impossible to absolutely precisely synchronize in time with the blockchain, so there is an allowable time delay value. If you get into this gap, the "expired" may be incorrectly set. - T: double spending for external withdrawals or unnecessary internal withdrawals - S: check expiration taking into account time delay -- D: check expiration taking into account time delay \ No newline at end of file +- D: check expiration taking into account time delay + +#### Repetitive failed transactions burning fees +- P: with periodic withdrawal cycles, there may be situations where the transaction fails every time. + For example, when withdrawing to an uninitialized cold wallet with the bounce flag. +- T: constant burning of a certain amount of TON on fees +- S: additional checks to predict the success of the transaction and additional messages in the audit log +- D: made an additional check on the state of the cold wallet and checking the bounce flag for withdrawal \ No newline at end of file diff --git a/todo_list.md b/todo_list.md index 7dc8edd..e65668d 100644 --- a/todo_list.md +++ b/todo_list.md @@ -48,7 +48,8 @@ - [x] Add filling deposit with bounce to test plan - [x] Update to tonutils-go 1.6.2 - [x] Process masterchain addresses for external incomes -- [ ] Cold wallet withdrawal fix +- [x] Cold wallet withdrawal fix +- [ ] Add hysteresis to cold wallet withdrawal - [ ] Add user id to notifications - [ ] Add transaction hash to notifications - [ ] Support DNS names in recipient address From a7ed0e0f72dc88cb5a233e6d88687e888cd9acd6 Mon Sep 17 00:00:00 2001 From: gobicycle Date: Sun, 2 Jul 2023 22:02:50 +0300 Subject: [PATCH 4/9] cold wallet withdrawal hysteresis --- README.md | 50 ++++++++++++++++++++++-------------- config/config.go | 42 +++++++++++++++++++++++------- core/withdrawal_processor.go | 4 +-- manual_testing_plan.md | 5 ++-- threat_model.md | 11 +++++++- todo_list.md | 1 + 6 files changed, 80 insertions(+), 33 deletions(-) diff --git a/README.md b/README.md index 097e8f4..6ee3e39 100644 --- a/README.md +++ b/README.md @@ -70,31 +70,43 @@ For more information on Jettons compatibility, see [Jettons compatibility](/jett ## Deployment ### Configurable parameters -| ENV variable | Description | -|------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| -| `LITESERVER` | IP and port of lite server, example: `185.86.76.183:5815` | -| `LITESERVER_KEY` | public key of lite server `5v2dHtSclsGsZVbNVwTj4hQDso5xvQjzL/yPEHJevHk=`.
Be careful with base64 encoding and ENV var. Use '' | -| `SEED` | seed phrase for main hot wallet. 24 words compatible with standard TON wallets | -| `DB_URI` | URI for DB connection, example:
`postgresql://db_user:db_password@localhost:5432/payment_processor` | -| `API_HOST` | host for REST API, example `localhost:8081`, default `0.0.0.0:8081` | -| `API_TOKEN` | Bearer token for REST API, example `123` | -| `IS_TESTNET` | `true` if service works in TESTNET, `false` - for MAINNET. Default: `true`. | -| `JETTONS` | list of Jettons, processed by service in format:
`JETTON_SYMBOL_1:MASTER_CONTRACT_ADDR_1:hot_wallet_max_balance:min_withdrawal_amount, JETTON_SYMBOL_2:MASTER_CONTRACT_ADDR_2:hot_wallet_max_balance:min_withdrawal_amount`,
example: `TGR:kQCKt2WPGX-fh0cIAz38Ljd_OKQjoZE_cqk7QrYGsNP6wfP0:1000000:100000` | -| `TON_CUTOFFS` | cutoffs in nanoTONs in format:
`hot_wallet_min_balance:hot_wallet_max_balance:min_withdrawal_amount`,
example `1000000000:100000000000:1000000000` | -| `COLD_WALLET` | cold-wallet address, example `kQCdyiS-fIV9UVfI9Phswo4l2MA-hm8YseH3XZ_YiH9Y1ufw`. If cold wallet is not active - use non-bounceable address (use https://ton.org/address for convert) | -| `DEPOSIT_SIDE_BALANCE` | `true` - service calculates total income for user by deposit incoming, `false` - by hot wallet incoming. Default: `true`. | -| `QUEUE_ENABLED` | `true` - service sends incoming notifications to queue, `false` - sending disabled. Default: `false`. | -| `QUEUE_URI` | URI for queue client connection, example `amqp://guest:guest@payment_rabbitmq:5672/` | -| `QUEUE_NAME` | name of exchange | -| `WEBHOOK_ENDPOINT` | endpoint to send webhooks, example: `http://hostname:3333/webhook`. If the value is not set, then webhooks are not sent. | -| `WEBHOOK_TOKEN` | Bearer token for webhook request. If not set then not used. | -| `ALLOWABLE_LAG` | allowable time lag between service time and last block time in seconds, default: 15 | +| ENV variable | Description | +|------------------------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| `LITESERVER` | IP and port of lite server, example: `185.86.76.183:5815` | +| `LITESERVER_KEY` | public key of lite server `5v2dHtSclsGsZVbNVwTj4hQDso5xvQjzL/yPEHJevHk=`.
Be careful with base64 encoding and ENV var. Use '' | +| `SEED` | seed phrase for main hot wallet. 24 words compatible with standard TON wallets | +| `DB_URI` | URI for DB connection, example:
`postgresql://db_user:db_password@localhost:5432/payment_processor` | +| `API_HOST` | host for REST API, example `localhost:8081`, default `0.0.0.0:8081` | +| `API_TOKEN` | Bearer token for REST API, example `123` | +| `IS_TESTNET` | `true` if service works in TESTNET, `false` - for MAINNET. Default: `true`. | +| `JETTONS` | list of Jettons, processed by service in format:
`JETTON_SYMBOL_1:MASTER_CONTRACT_ADDR_1:hot_wallet_max_balance:min_withdrawal_amount:hot_wallet_residual_balance, JETTON_SYMBOL_2:MASTER_CONTRACT_ADDR_2:hot_wallet_max_balance:min_withdrawal_amount:hot_wallet_residual_balance`,
example: `TGR:kQCKt2WPGX-fh0cIAz38Ljd_OKQjoZE_cqk7QrYGsNP6wfP0:1000000:100000` | +| `TON_CUTOFFS` | cutoffs in nanoTONs in format:
`hot_wallet_min_balance:hot_wallet_max_balance:min_withdrawal_amount:hot_wallet_residual_balance`,
example `1000000000:100000000000:1000000000:95000000000` | +| `COLD_WALLET` | cold-wallet address, example `kQCdyiS-fIV9UVfI9Phswo4l2MA-hm8YseH3XZ_YiH9Y1ufw`. If cold wallet is not active - use non-bounceable address (use https://ton.org/address for convert) | +| `DEPOSIT_SIDE_BALANCE` | `true` - service calculates total income for user by deposit incoming, `false` - by hot wallet incoming. Default: `true`. | +| `QUEUE_ENABLED` | `true` - service sends incoming notifications to queue, `false` - sending disabled. Default: `false`. | +| `QUEUE_URI` | URI for queue client connection, example `amqp://guest:guest@payment_rabbitmq:5672/` | +| `QUEUE_NAME` | name of exchange | +| `WEBHOOK_ENDPOINT` | endpoint to send webhooks, example: `http://hostname:3333/webhook`. If the value is not set, then webhooks are not sent. | +| `WEBHOOK_TOKEN` | Bearer token for webhook request. If not set then not used. | +| `ALLOWABLE_LAG` | allowable time lag between service time and last block time in seconds, default: 15 | **! Be careful with `IS_TESTNET` variable.** This does not guarantee that a testnet node is being used. It is only for address checking purposes. There are also internal service settings (fees and timeouts) that are specified in the source code in the [Config](/config/config.go) package. Calibration parameters recommendations in [Technical notes](/technical_notes.md). +#### `hot_wallet_residual_balance` and `hot_wallet_max_balance` + +In order to avoid triggering a withdrawal to a cold wallet with each receipt of funds, a hysteresis is introduced. +`hot_wallet_max_balance` - this is the amount at which the withdrawal from the hot wallet to the cold one will be triggered +`hot_wallet_residual_balance` is the amount that will remain on the hot wallet after the withdrawal + +`hot_wallet_max_balance` must be greater than `hot_wallet_residual_balance` + +If the `hot_wallet_residual_balance` is not set, then it is calculated using the formula: +`hot_wallet_residual_balance` = `hot_wallet_max_balance` * `hysteresis`, where hysteresis is a hardcoded value +(at the time of writing this is 0.95) + ### Service deploy **Do not use same `.env` file for `payment-processor` and other services!** diff --git a/config/config.go b/config/config.go index 4cfb5d3..0997ada 100644 --- a/config/config.go +++ b/config/config.go @@ -16,6 +16,8 @@ var ( JettonTransferTonAmount = tlb.FromNanoTONU(100_000_000) JettonForwardAmount = tlb.FromNanoTONU(20_000_000) // must be < JettonTransferTonAmount + DefaultHotWalletHysteresis = decimal.NewFromFloat(0.95) // `hot_wallet_residual_balance` = `hot_wallet_max_balance` * `hysteresis` + ExternalMessageLifetime = 50 * time.Second ExternalWithdrawalPeriod = 80 * time.Second // must be ExternalWithdrawalPeriod > ExternalMessageLifetime and some time for balance update @@ -62,12 +64,14 @@ type Jetton struct { Master *address.Address WithdrawalCutoff *big.Int HotWalletMaxCutoff *big.Int + HotWalletResidual *big.Int } type Cutoffs struct { - HotWalletMin *big.Int - HotWalletMax *big.Int - Withdrawal *big.Int + HotWalletMin *big.Int + HotWalletMax *big.Int + Withdrawal *big.Int + HotWalletResidual *big.Int } func GetConfig() { @@ -119,7 +123,7 @@ func parseJettonString(s string) map[string]Jetton { jettons := strings.Split(s, ",") for _, j := range jettons { data := strings.Split(j, ":") - if len(data) != 4 { + if len(data) != 4 && len(data) != 5 { log.Fatalf("invalid jetton data") } cur := data[0] @@ -135,10 +139,20 @@ func parseJettonString(s string) map[string]Jetton { if err != nil { log.Fatalf("invalid %v jetton withdrawal cutoff: %v", data[0], err) } + + residual := maxCutoff.Mul(DefaultHotWalletHysteresis) + if len(data) == 5 { + residual, err = decimal.NewFromString(data[4]) + if err != nil { + log.Fatalf("invalid hot_wallet_residual_balance parameter: %v", err) + } + } + res[cur] = Jetton{ Master: addr, WithdrawalCutoff: withdrawalCutoff.BigInt(), HotWalletMaxCutoff: maxCutoff.BigInt(), + HotWalletResidual: residual.BigInt(), } } return res @@ -146,8 +160,8 @@ func parseJettonString(s string) map[string]Jetton { func parseTonString(s string) Cutoffs { data := strings.Split(s, ":") - if len(data) != 3 { - log.Fatalf("invalid jetton data") + if len(data) != 3 && len(data) != 4 { + log.Fatalf("invalid TON cuttofs") } hotWalletMin, err := decimal.NewFromString(data[0]) if err != nil { @@ -164,9 +178,19 @@ func parseTonString(s string) Cutoffs { if hotWalletMin.Cmp(hotWalletMax) == 1 { log.Fatalf("TON hot wallet max cutoff must be greater than TON hot wallet min cutoff") } + + residual := hotWalletMax.Mul(DefaultHotWalletHysteresis) + if len(data) == 4 { + residual, err = decimal.NewFromString(data[3]) + if err != nil { + log.Fatalf("invalid hot_wallet_residual_balance parameter: %v", err) + } + } + return Cutoffs{ - HotWalletMin: hotWalletMin.BigInt(), - HotWalletMax: hotWalletMax.BigInt(), - Withdrawal: withdrawal.BigInt(), + HotWalletMin: hotWalletMin.BigInt(), + HotWalletMax: hotWalletMax.BigInt(), + Withdrawal: withdrawal.BigInt(), + HotWalletResidual: residual.BigInt(), } } diff --git a/core/withdrawal_processor.go b/core/withdrawal_processor.go index 346aca1..fff43be 100644 --- a/core/withdrawal_processor.go +++ b/core/withdrawal_processor.go @@ -631,7 +631,7 @@ func (p *WithdrawalsProcessor) makeColdWalletWithdrawals(ctx context.Context) er if err != nil { return err } - jettonAmount.Sub(jettonBalance, config.Config.Jettons[cur].HotWalletMaxCutoff) + jettonAmount.Sub(jettonBalance, config.Config.Jettons[cur].HotWalletResidual) tonBalance.Sub(tonBalance, config.JettonTransferTonAmount.NanoTON()) req := WithdrawalRequest{ Currency: jw.Currency, @@ -665,7 +665,7 @@ func (p *WithdrawalsProcessor) makeColdWalletWithdrawals(ctx context.Context) er if err != nil { return err } - tonAmount.Sub(tonBalance, config.Config.Ton.HotWalletMax) + tonAmount.Sub(tonBalance, config.Config.Ton.HotWalletResidual) req := WithdrawalRequest{ Currency: TonSymbol, Amount: NewCoins(tonAmount), diff --git a/manual_testing_plan.md b/manual_testing_plan.md index 768af87..ac9c37a 100644 --- a/manual_testing_plan.md +++ b/manual_testing_plan.md @@ -276,9 +276,10 @@ Template: 3. -[ ] Checked - TEST : Replenish the deposit with TONs and Jettons so that as a result the amount on the hot wallet is greater - than `hot_wallet_max_balance`. Check withdrawals in DB + than `hot_wallet_max_balance`. Try with and without `hot_wallet_residual_balance` parameter. Check withdrawals in DB - RESULT : You must find new withdrawal in `withdrawal_requests` table with `is_internal=true`. And final status - must correlate with explorer. Withdrawal amount must correlate with hysteresis formula. + must correlate with explorer. Withdrawal amount must correlate with hysteresis formula + (and `hot_wallet_residual_balance` parameter). - COMMENT : ### Deploy diff --git a/threat_model.md b/threat_model.md index 5d3c3a3..34f14c3 100644 --- a/threat_model.md +++ b/threat_model.md @@ -133,4 +133,13 @@ If TONs arrive at the wallet address at this time, the message will be applied a For example, when withdrawing to an uninitialized cold wallet with the bounce flag. - T: constant burning of a certain amount of TON on fees - S: additional checks to predict the success of the transaction and additional messages in the audit log -- D: made an additional check on the state of the cold wallet and checking the bounce flag for withdrawal \ No newline at end of file +- D: made an additional check on the state of the cold wallet and checking the bounce flag for withdrawal + +#### Too frequent withdrawals from a hot wallet to a cold wallet +- P: if you set only the maximum cutoff for funds on the hot wallet, then the withdrawal to the cold wallet will occur + if this amount is exceeded, even if the amount of the excess is less than the amount of the withdrawal fee +- T: there may be withdrawals of the amount of funds at which the amount of funds is unreasonably small, + which will lead to unnecessary burning of funds on fees +- S: it is necessary to set some delta between the amount of triggering the withdrawal to the cold wallet and the + amount that will remain after the withdrawal +- D: one more parameter has been added to the cutoffs - `hot_wallet_residual_balance` \ No newline at end of file diff --git a/todo_list.md b/todo_list.md index e65668d..17e3b46 100644 --- a/todo_list.md +++ b/todo_list.md @@ -69,3 +69,4 @@ - [ ] Automatic migrations - [ ] SDK - [ ] migration from blueprint to openapi +- [ ] refactor config and cutoff parameters From 3fadaa3dd69faba560550ab10006f608f498cce9 Mon Sep 17 00:00:00 2001 From: gobicycle Date: Sun, 2 Jul 2023 23:38:48 +0300 Subject: [PATCH 5/9] user id and transaction hash in notification body --- README.md | 8 ++++++-- core/block_scanner.go | 19 +++++++++++++++++-- core/models.go | 8 ++++++-- db/db.go | 26 ++++++++++++++++---------- manual_testing_plan.md | 2 +- todo_list.md | 7 ++++--- 6 files changed, 50 insertions(+), 20 deletions(-) diff --git a/README.md b/README.md index 6ee3e39..c8f29a6 100644 --- a/README.md +++ b/README.md @@ -148,7 +148,9 @@ Message format when `DEPOSIT_SIDE_BALANCE` == true: "time": 12345678, "amount":"100", "source_address":"0QAOp2OZwWdkF5HhJ0WVDspgh6HhpmHyQ3cBuBmfJ4q_AIVe", - "comment":"hello" + "comment":"hello", + "tx_hash": "f9b9e7efd3a38da318a894576499f0b6af5ca2da97ccd15c5f1d291a808a0ebf", + "user_id": "123" } ``` @@ -158,7 +160,9 @@ from the deposit): { "deposit_address":"0QCdsj-u39qVlfYdpPKuAY0hTe5VIsiJcpB5Rx4tOUOyBFhL", "time": 12345678, - "amount":"200" + "amount":"200", + "tx_hash": "f9b9e7efd3a38da318a894576499f0b6af5ca2da97ccd15c5f1d291a808a0ebf", + "user_id": "123" } ``` diff --git a/core/block_scanner.go b/core/block_scanner.go index 66f782a..172c9d5 100644 --- a/core/block_scanner.go +++ b/core/block_scanner.go @@ -58,6 +58,8 @@ type incomeNotification struct { Amount string `json:"amount"` Source string `json:"source_address,omitempty"` Comment string `json:"comment,omitempty"` + UserID string `json:"user_id"` + TxHash string `json:"tx_hash"` } func NewBlockScanner( @@ -133,14 +135,14 @@ func (s *BlockScanner) pushNotifications(e BlockEvents) error { if config.Config.IsDepositSideCalculation { for _, ei := range e.ExternalIncomes { - err := s.pushNotification(ei.To, ei.Amount, ei.Utime, ei.From, ei.FromWorkchain, ei.Comment) + err := s.pushNotification(ei.To, ei.Amount, ei.Utime, ei.From, ei.FromWorkchain, ei.Comment, ei.TxHash) if err != nil { return err } } } else { for _, ii := range e.InternalIncomes { - err := s.pushNotification(ii.From, ii.Amount, ii.Utime, nil, nil, "") + err := s.pushNotification(ii.From, ii.Amount, ii.Utime, nil, nil, "", ii.TxHash) if err != nil { return err } @@ -156,16 +158,23 @@ func (s *BlockScanner) pushNotification( from []byte, fromWorkchain *int32, comment string, + txHash []byte, ) error { owner := s.db.GetOwner(addr) if owner != nil { addr = *owner } + userID, ok := s.db.GetUserID(addr) + if !ok { + return fmt.Errorf("not found UserID for deposit %s", addr.ToUserFormat()) + } notification := incomeNotification{ Deposit: addr.ToUserFormat(), Amount: amount.String(), Timestamp: int64(timestamp), Comment: comment, + UserID: userID, + TxHash: fmt.Sprintf("%x", txHash), } if len(from) == 32 && fromWorkchain != nil { // supports only std address @@ -433,6 +442,7 @@ func convertUnknownJettonTxs(txs []*tlb.Transaction, addr Address, amount *big.I Lt: tx.LT, To: addr, Amount: ZeroCoins(), + TxHash: tx.Hash, }) } @@ -442,6 +452,7 @@ func convertUnknownJettonTxs(txs []*tlb.Transaction, addr Address, amount *big.I Lt: txs[0].LT, To: addr, Amount: NewCoins(amount), + TxHash: txs[0].Hash, }) } return incomes, nil @@ -746,6 +757,7 @@ func (s *BlockScanner) processTonHotWalletInternalInMsg(tx *tlb.Transaction) (Ev Amount: NewCoins(inMsg.Amount.NanoTON()), Memo: inMsg.Comment(), IsFailed: false, + TxHash: tx.Hash, } success, err := checkTxForSuccess(tx) if err != nil { @@ -777,6 +789,7 @@ func (s *BlockScanner) processTonHotWalletInternalInMsg(tx *tlb.Transaction) (Ev Amount: income.Amount, Memo: income.Comment, IsFailed: false, + TxHash: tx.Hash, }) } } @@ -854,6 +867,7 @@ func (s *BlockScanner) processTonDepositWalletInternalInMsg(tx *tlb.Transaction) To: dstAddr, Amount: NewCoins(inMsg.Amount.NanoTON()), Comment: inMsg.Comment(), + TxHash: tx.Hash, }) } return events, nil @@ -927,6 +941,7 @@ func (s *BlockScanner) processJettonDepositOutMsgs(tx *tlb.Transaction) (Events, To: srcAddr, Amount: notify.Amount, Comment: notify.Comment, + TxHash: tx.Hash, }) knownIncomeAmount.Add(knownIncomeAmount, notify.Amount.BigInt()) } diff --git a/core/models.go b/core/models.go index ebd3a5d..8572042 100644 --- a/core/models.go +++ b/core/models.go @@ -131,8 +131,9 @@ func AddressMustFromTonutilsAddress(addr *address.Address) Address { } type AddressInfo struct { - Type WalletType - Owner *Address + Type WalletType + Owner *Address + UserID string } type JettonWallet struct { @@ -222,6 +223,7 @@ type InternalIncome struct { Amount Coins Memo string IsFailed bool + TxHash []byte } type ExternalIncome struct { @@ -232,6 +234,7 @@ type ExternalIncome struct { To Address Amount Coins Comment string + TxHash []byte } type Events struct { @@ -297,6 +300,7 @@ type storage interface { SaveJettonWallet(ctx context.Context, ownerAddress Address, walletData WalletData, notSaveOwner bool) error GetWalletType(address Address) (WalletType, bool) GetOwner(address Address) *Address + GetUserID(address Address) (string, bool) GetWalletTypeByTonutilsAddress(address *address.Address) (WalletType, bool) SaveParsedBlockData(ctx context.Context, events BlockEvents) error GetTonInternalWithdrawalTasks(ctx context.Context, limit int) ([]InternalWithdrawalTask, error) diff --git a/db/db.go b/db/db.go index 3935eec..21427b8 100644 --- a/db/db.go +++ b/db/db.go @@ -57,6 +57,11 @@ func (c *Connection) GetWalletType(address core.Address) (core.WalletType, bool) return info.Type, ok } +func (c *Connection) GetUserID(address core.Address) (string, bool) { + info, ok := c.addressBook.get(address) + return info.UserID, ok +} + // GetOwner returns owner for jetton deposit from address book and nil for other types func (c *Connection) GetOwner(address core.Address) *core.Address { info, ok := c.addressBook.get(address) @@ -103,7 +108,7 @@ func (c *Connection) SaveTonWallet(ctx context.Context, walletData core.WalletDa if err != nil { return err } - c.addressBook.put(walletData.Address, core.AddressInfo{Type: walletData.Type, Owner: nil}) + c.addressBook.put(walletData.Address, core.AddressInfo{Type: walletData.Type, Owner: nil, UserID: walletData.UserID}) return nil } @@ -186,7 +191,7 @@ func (c *Connection) SaveJettonWallet( // cold wallets excluded from address book c.addressBook.put(ownerAddress, core.AddressInfo{Type: core.JettonOwner, Owner: nil}) } - c.addressBook.put(walletData.Address, core.AddressInfo{Type: walletData.Type, Owner: &ownerAddress}) + c.addressBook.put(walletData.Address, core.AddressInfo{Type: walletData.Type, Owner: &ownerAddress, UserID: walletData.UserID}) return nil } @@ -258,12 +263,13 @@ func (c *Connection) GetJettonOwnersAddresses( func (c *Connection) LoadAddressBook(ctx context.Context) error { res := make(map[core.Address]core.AddressInfo) var ( - addr core.Address - t core.WalletType + addr core.Address + t core.WalletType + userID string ) rows, err := c.client.Query(ctx, ` - SELECT address, type + SELECT address, type, user_id FROM payments.ton_wallets `) if err != nil { @@ -271,15 +277,15 @@ func (c *Connection) LoadAddressBook(ctx context.Context) error { } defer rows.Close() for rows.Next() { - err = rows.Scan(&addr, &t) + err = rows.Scan(&addr, &t, &userID) if err != nil { return err } - res[addr] = core.AddressInfo{Type: t, Owner: nil} + res[addr] = core.AddressInfo{Type: t, Owner: nil, UserID: userID} } rows, err = c.client.Query(ctx, ` - SELECT jw.address, jw.type, tw.address + SELECT jw.address, jw.type, tw.address, jw.user_id FROM payments.jetton_wallets jw LEFT JOIN payments.ton_wallets tw ON jw.subwallet_id = tw.subwallet_id `) @@ -289,11 +295,11 @@ func (c *Connection) LoadAddressBook(ctx context.Context) error { defer rows.Close() for rows.Next() { var owner core.Address - err = rows.Scan(&addr, &t, &owner) + err = rows.Scan(&addr, &t, &owner, &userID) if err != nil { return err } - res[addr] = core.AddressInfo{Type: t, Owner: &owner} + res[addr] = core.AddressInfo{Type: t, Owner: &owner, UserID: userID} } c.addressBook.addresses = res diff --git a/manual_testing_plan.md b/manual_testing_plan.md index ac9c37a..2d6ab8f 100644 --- a/manual_testing_plan.md +++ b/manual_testing_plan.md @@ -254,7 +254,7 @@ Template: 28. -[ ] Checked - TEST : Replenish the Jetton deposit with zero forward amount and check it by - `/v1/history{?user_id,currency,limit,offset}` method. + `/v1/history{?user_id,currency,limit,offset}` method. - RESULT : The sender's address must be not presented in the history. - COMMENT : diff --git a/todo_list.md b/todo_list.md index 17e3b46..d47f4d5 100644 --- a/todo_list.md +++ b/todo_list.md @@ -49,9 +49,10 @@ - [x] Update to tonutils-go 1.6.2 - [x] Process masterchain addresses for external incomes - [x] Cold wallet withdrawal fix -- [ ] Add hysteresis to cold wallet withdrawal -- [ ] Add user id to notifications -- [ ] Add transaction hash to notifications +- [x] Add hysteresis to cold wallet withdrawal +- [x] Add user id to notifications +- [x] Add transaction hash to notifications +- [ ] Save tx hash to DB - [ ] Support DNS names in recipient address - [ ] Jetton threat model - [ ] TNX compatibility test From 71e451fc240c7a933bce7103aafac9e9ef7e0bde Mon Sep 17 00:00:00 2001 From: gobicycle Date: Tue, 4 Jul 2023 01:07:54 +0300 Subject: [PATCH 6/9] manual tests --- manual_testing_plan.md | 22 +++++++++++----------- todo_list.md | 1 + 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/manual_testing_plan.md b/manual_testing_plan.md index 2d6ab8f..0db836e 100644 --- a/manual_testing_plan.md +++ b/manual_testing_plan.md @@ -64,7 +64,7 @@ Template: ### API -1. -[ ] Checked +1. -[x] Checked - TEST : Use `/v1/address/new` method (few for TONs and few for Jettons for different users). Check new addresses in DB - RESULT : You must receive different addresses in user-friendly format with `bounce = false` flag and @@ -72,23 +72,23 @@ Template: owner address. - COMMENT : -2. -[ ] Checked +2. -[x] Checked - TEST : Use `/v1/address/all{?user_id}` method and compare with addresses created at 1. And check it by DB - RESULT : All addresses must be received and equal to those created earlier - COMMENT : -3. -[ ] Checked +3. -[x] Checked - TEST : Check `/v1/income{?user_id}` for new empty deposits - RESULT : Income must be zero. The addresses must match the addresses obtained by method `/v1/address/all{?user_id}`. - COMMENT : -4. -[ ] Checked +4. -[x] Checked - TEST : Make some payments at deposits and check it by `/v1/income{?user_id}` method with different `DEPOSIT_SIDE_BALANCE` env var - RESULT : Income must correlate with payments sum - COMMENT : -5. -[ ] Checked +5. -[x] Checked - TEST : Make some withdrawals by `/v1/withdrawal/send` method for TONs and Jettons with amount > hot wallet balance and check it by `/v1/withdrawal/status{?id}` few times. Check status of withdrawals by transaction explorer (e.g. https://testnet.tonapi.io/ or https://tonapi.io/). Check withdrawal in DB. @@ -96,7 +96,7 @@ Template: There is no any correlated messages in `external withdrawals` table. - COMMENT : -6. -[ ] Checked +6. -[x] Checked - TEST : Make some withdrawals by `/v1/withdrawal/send` method for TONs and check it by `/v1/withdrawal/status{?id}` few times and try to catch all statuses: `pending`, `processing`, `processed`. Check status of withdrawals by transaction explorer (e.g. https://testnet.tonapi.io/ or https://tonapi.io/). Check withdrawal in DB. @@ -113,7 +113,7 @@ Template: There is no any correlated messages in `external withdrawals` table. - COMMENT : -8. -[ ] Checked +8. -[x] Checked - TEST : Make some withdrawals by `/v1/withdrawal/send` method for Jettons with deployed Jetton hot wallets and check it by `/v1/withdrawal/status{?id}` few times and try to catch all statuses: `pending`, `processing`, `processed`. Check status of withdrawals by transaction explorer @@ -239,7 +239,7 @@ Template: should always be displayed. - COMMENT : -26. -[ ] Checked +26. -[x] Checked - TEST : Replenish the TON deposit from the masterchain wallet and check it by `/v1/history{?user_id,currency,limit,offset}` method. - RESULT : The sender's address must be displayed correctly in the history. @@ -274,7 +274,7 @@ Template: - RESULT : There should be no missing blocks in the DB. - COMMENT : -3. -[ ] Checked +3. -[x] Checked - TEST : Replenish the deposit with TONs and Jettons so that as a result the amount on the hot wallet is greater than `hot_wallet_max_balance`. Try with and without `hot_wallet_residual_balance` parameter. Check withdrawals in DB - RESULT : You must find new withdrawal in `withdrawal_requests` table with `is_internal=true`. And final status @@ -316,7 +316,7 @@ Template: payment activity - COMMENT : -6. -[ ] Checked +6. -[x] Checked - TEST : Start `payment-processor` with `WEBHOOK_ENDPOINT=http://localhost:3333/webhook` env var. Start test webserver from `cmd/testwebhook/main.go`. Make some payments to deposits. Check payments data at webserver side. Add env variable `WEBHOOK_TOKEN=123` and restart `payment-processor`. Make some payments @@ -325,7 +325,7 @@ Template: `WEBHOOK_TOKEN` is not set. - COMMENT : -7. -[ ] Checked +7. -[x] Checked - TEST : Start `payment-processor` with webhooks. Make Jetton payment to deposits with zero froward amount. Check payments data at webserver side. - RESULT : The sender's address must be not presented. diff --git a/todo_list.md b/todo_list.md index d47f4d5..7b99730 100644 --- a/todo_list.md +++ b/todo_list.md @@ -52,6 +52,7 @@ - [x] Add hysteresis to cold wallet withdrawal - [x] Add user id to notifications - [x] Add transaction hash to notifications +- [ ] Avoid blocking withdrawals to an address if there is a very large amount in the queue for withdrawals to this address - [ ] Save tx hash to DB - [ ] Support DNS names in recipient address - [ ] Jetton threat model From e2246a16e937a247c0208ace78fa4b15f82c979a Mon Sep 17 00:00:00 2001 From: gobicycle Date: Wed, 5 Jul 2023 00:34:53 +0300 Subject: [PATCH 7/9] manual tests --- manual_testing_plan.md | 48 +++++++++++++++++++++--------------------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/manual_testing_plan.md b/manual_testing_plan.md index 0db836e..2b4e67b 100644 --- a/manual_testing_plan.md +++ b/manual_testing_plan.md @@ -7,51 +7,51 @@ Template: ### Initialization -1. -[ ] Checked +1. -[x] Checked - TEST : Run with not deployed (and zero balance) hot wallet (new seed phrase) - RESULT : There must be an insufficient balance error - COMMENT : -2. -[ ] Checked +2. -[x] Checked - TEST : Run with uninit hot wallet with balance > minimum balance - RESULT : Hot wallet must be initialized at first start of service - COMMENT : -3. -[ ] Checked +3. -[x] Checked - TEST : Run with new seed phrase when hot wallet already exist in DB - RESULT : There must be an incorrect seed phrase error - COMMENT : -4. -[ ] Checked +4. -[x] Checked - TEST : Run service with empty DB and stop after few minutes. Check time of first and last block in `block_data` table - RESULT : Time `saved_at` and `gen_utime` must correlate with system time - COMMENT : -5. -[ ] Checked +5. -[x] Checked - TEST : Run with nonexist hot jetton wallet and receive external jetton transfer at jetton deposit (> minimal withdrawal amount) - RESULT : Jetton hot wallet must be initialized by Jetton withdrawal from deposit, if jetton deposit successfully initialized (it depends on transfer sender) - COMMENT : -6. -[ ] Checked +6. -[x] Checked - TEST : Run with testnet cold wallet address at mainnet (`IS_TESTNET=false`) - RESULT : There must be "Can not use testnet cold wallet address for mainnet" error - COMMENT : -7. -[ ] Checked +7. -[x] Checked - TEST : Run service with empty `JETTONS` env variable - RESULT : Service must start and process TONs - COMMENT : -8. -[ ] Checked +8. -[x] Checked - TEST : Run service with `JETTONS` env variable with different currencies and same master contract address. Like `TGR:ABC...,FNZ:ABC...`. - RESULT : Service must stop. Must be address duplication error message in audit log. - COMMENT : -9. -[ ] Checked +9. -[x] Checked - TEST : Run service with one `JETTONS` env variable, then rename currency for one of Jetton and restart. Like `TGR:ABC...,FNZ:CDE...` -> `SCALE:ABC...,FNZ:CDE...`. - RESULT : Service must stop. Must be address duplication error message in audit log. @@ -105,7 +105,7 @@ Template: as final state. - COMMENT : -7. -[ ] Checked +7. -[x] Checked - TEST : Make some withdrawals by `/v1/withdrawal/send` method for Jettons with not deployed Jetton hot wallets and withdrawals by `/v1/withdrawal/status{?id}` few times. Check status of withdrawals by transaction explorer (e.g. https://testnet.tonapi.io/ or https://tonapi.io/). Check withdrawal in DB. @@ -123,35 +123,35 @@ Template: as final state. - COMMENT : -9. -[ ] Checked +9. -[x] Checked - TEST : Start the service after some downtime. Check sync status by `/v1/system/sync` few times. - RESULT : Start status should be `"is_synced": false` then become `"is_synced": true` - COMMENT : -10. -[ ] Checked +10. -[x] Checked - TEST : Start service with `IS_TESTNET=true` env var. Make some withdrawals by `/v1/withdrawal/send` method to TESTNET and MAINNET user-friendly form addresses. - RESULT : All withdrawals must be accepted - COMMENT : -11. -[ ] Checked +11. -[x] Checked - TEST : Start service with `IS_TESTNET=false` env var. Make some withdrawals by `/v1/withdrawal/send` method to TESTNET user-friendly form addresses. - RESULT : All withdrawals must be rejected - COMMENT : -12. -[ ] Checked +12. -[x] Checked - TEST : Make some withdrawals by `/v1/withdrawal/send` method to service internal addresses (hot wallet, jetton hot wallet, owner, ton deposit, jetton deposit). - RESULT : All withdrawals must be rejected - COMMENT : -13. -[ ] Checked +13. -[x] Checked - TEST : Try all methods with auth using wrong token - RESULT : All requests must be rejected - COMMENT : -14. -[ ] Checked +14. -[x] Checked - TEST : Make TON and Jetton withdrawal to -1 workchain - RESULT : Withdrawals must be rejected - COMMENT : @@ -219,7 +219,7 @@ Template: In `service_withdrawal_request` DB table must be `processed = true` - COMMENT : -23. -[ ] Checked +23. -[x] Checked - TEST : Set some Jetton in `JETTONS` env variable. Start service to init jetton hot wallet in DB. Remove Jetton from env variable and restart. Try `/v1/address/new`, `/v1/withdrawal/send` for removed Jetton. - RESULT : Must be currency error for `/v1/address/new`, `/v1/withdrawal/send`. @@ -232,7 +232,7 @@ Template: - RESULT : Removed Jetton should not appear in `/v1/address/all`, `/v1/income`. - COMMENT : Not implemented yet -25. -[ ] Checked +25. -[x] Checked - TEST : Make some payments at deposits and check it by `/v1/history{?user_id,currency,limit,offset}` method with different `DEPOSIT_SIDE_BALANCE` env var - RESULT : Incomes must correlate with payments and DB `external_incomes` table. The history on the deposits side @@ -245,14 +245,14 @@ Template: - RESULT : The sender's address must be displayed correctly in the history. - COMMENT : -27. -[ ] Checked +27. -[x] Checked - TEST : Replenish the TON deposit (when it in nonexist status) with a bounceable message and check it by `/v1/history{?user_id,currency,limit,offset}` method. Also check logs. - RESULT : The bounced payment should not be in the history. There should be no errors in the logs, only a warning about a bounced message. - COMMENT : -28. -[ ] Checked +28. -[x] Checked - TEST : Replenish the Jetton deposit with zero forward amount and check it by `/v1/history{?user_id,currency,limit,offset}` method. - RESULT : The sender's address must be not presented in the history. @@ -260,7 +260,7 @@ Template: ### Internal logic -1. -[ ] Checked +1. -[x] Checked - TEST : Replenish the deposit with TONs and Jettons so that as a result the amount on the hot wallet is greater than `hot_wallet_max_balance` when cold wallet is not active and cold wallet address in non-bounceable format. Check withdrawals in DB @@ -284,19 +284,19 @@ Template: ### Deploy -1. -[ ] Checked +1. -[x] Checked - TEST : Build docker images and start `payment-postgres`, `payment-processor` services using README.md instructions Check availability and functionality of service. - RESULT : The API must be accessible and functional - COMMENT : -2. -[ ] Checked +2. -[x] Checked - TEST : Start optional `payment-grafana` service using README.md instructions Check availability and functionality of service. - RESULT : The `payments` Grafana dashboard must be accessible and show DB data - COMMENT : -3. -[ ] Checked +3. -[x] Checked - TEST : Start `payment-processor` with `QUEUE_ENABLED=true` env var and optional `payment-rabbitmq` service using README.md instructions. Make some payments to deposits. Check availability and functionality of service by RabbitMQ dashboard From fc78a46f057e024e667a49d736f5470498865a93 Mon Sep 17 00:00:00 2001 From: gobicycle Date: Thu, 6 Jul 2023 00:36:56 +0300 Subject: [PATCH 8/9] manual tests --- manual_testing_plan.md | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/manual_testing_plan.md b/manual_testing_plan.md index 2b4e67b..e6c0c0b 100644 --- a/manual_testing_plan.md +++ b/manual_testing_plan.md @@ -156,19 +156,19 @@ Template: - RESULT : Withdrawals must be rejected - COMMENT : -15. -[ ] Checked +15. -[x] Checked - TEST : Make withdrawals by `/v1/withdrawal/service/ton` and `/v1/withdrawal/service/jetton` from unknown address and another network (testnet addr for mainnet address and -1 workchain address) - RESULT : Withdrawals must be rejected - COMMENT : -16. -[ ] Checked +16. -[x] Checked - TEST : Make withdrawal by `/v1/withdrawal/service/ton` and `/v1/withdrawal/service/jetton` from known internal but not Jetton deposit owner and not TON deposit address (hot wallet and Jetton wallet) - RESULT : Withdrawal must be rejected - COMMENT : -17. -[ ] Checked +17. -[x] Checked - TEST : Make TON withdrawal by `/v1/withdrawal/service/ton` from known Jetton deposit owner address. Check for unusual transactions in the database - RESULT : Withdrawal must be accepted. All TONs must be sent from Jetton deposit owner to hot wallet. There is no @@ -176,7 +176,7 @@ Template: must be `processed = true` - COMMENT : -18. -[ ] Checked +18. -[x] Checked - TEST : Make Jetton (not deposit Jetton type) withdrawal by `/v1/withdrawal/service/jetton` from known internal Jetton deposit owner address. Check for unusual transactions in the database - RESULT : Withdrawal must be accepted. Jettons must be sent from Jetton wallet to hot wallet. @@ -185,7 +185,7 @@ Template: - COMMENT : Should be zero forward TON amount in transfer message to prevent invoking notification message and incorrect interpretation hot wallet incoming message -19. -[ ] Checked +19. -[x] Checked - TEST : Make Jetton withdrawal by `/v1/withdrawal/service/jetton` from known internal TON deposit address. Check for unusual transactions in the database - RESULT : Withdrawal must be accepted. First, there must be a TON filling transaction from hot wallet to TON deposit. @@ -197,21 +197,21 @@ Template: TON deposit occurs through the Jetton wallet and is not detected by the block scanner as an internal TON withdrawal. The deposit balance on the hot wallet side is not replenished. -20. -[ ] Checked +20. -[x] Checked - TEST : Make Jetton (for deposit Jetton type) withdrawal by `/v1/withdrawal/service/jetton` from known internal Jetton deposit owner address. - RESULT : In `service_withdrawal_request` DB table should be `processed = true` and `filled=false` with zero balances. Must be warning about rejected withdrawal in audit log - COMMENT : -21. -[ ] Checked +21. -[x] Checked - TEST : Make TON withdrawal by `/v1/withdrawal/service/ton` from Jetton owner address with zero TON balance - RESULT : Withdrawal must be accepted. There should be audit log info about zero balance. There is no any deposit transactions (incomes/withdrawals) in the database and no messages from hot wallet. In `service_withdrawal_request` DB table must be `processed = true` - COMMENT : -22. -[ ] Checked +22. -[x] Checked - TEST : Make Jetton withdrawal by `/v1/withdrawal/service/jetton` from Jetton owner address and from TON deposit address with zero Jetton balance - RESULT : Withdrawal must be accepted. There should be audit log info about zero balance. There is no any deposit @@ -303,13 +303,13 @@ Template: - RESULT : Must be some message activity in RabbitMQ dashboard for exchange - COMMENT : -4. -[ ] Checked +4. -[x] Checked - TEST : Start `payment-test` service using technical_notes.md instructions with `CIRCULATION=false` env variable. Check availability and functionality of service by Grafana dashboard. - RESULT : Grafana must show prometheus metrics from `payment-test` service (deposit and total balances) - COMMENT : -5. -[ ] Checked +5. -[x] Checked - TEST : Start `payment-test` service using technical_notes.md instructions with `CIRCULATION=true` env variable. Check availability and functionality of service by Grafana dashboard. - RESULT : Grafana must show prometheus metrics from `payment-test` service (deposit and total balances) and @@ -333,7 +333,7 @@ Template: ### Stability test -1. -[ ] Checked +1. -[x] Checked - TEST : Start `payment-test` service using technical_notes.md instructions with `CIRCULATION=true` env variable for long time (with enough amount of test TONs on wallet). Periodically check availability and functionality of service by Grafana dashboard and docker logs. From 166fb0d40ffac614f720b5ddf551b67fc84d7296 Mon Sep 17 00:00:00 2001 From: gobicycle Date: Thu, 6 Jul 2023 09:00:47 +0300 Subject: [PATCH 9/9] fix test --- blockchain/blockchain_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/blockchain/blockchain_test.go b/blockchain/blockchain_test.go index a5c4886..709aee6 100644 --- a/blockchain/blockchain_test.go +++ b/blockchain/blockchain_test.go @@ -215,7 +215,7 @@ func Test_GetAccountCurrentState(t *testing.T) { func Test_DeployTonWallet(t *testing.T) { c := connect(t) seed := getSeed() - ctx, cancel := context.WithTimeout(context.Background(), time.Second*120) + ctx, cancel := context.WithTimeout(context.Background(), time.Second*200) defer cancel() amount := tlb.FromNanoTONU(100_000_000) mainWallet, _, _, err := c.GenerateDefaultWallet(seed, true) @@ -229,7 +229,7 @@ func Test_DeployTonWallet(t *testing.T) { if b.Cmp(amount.NanoTON()) != 1 || st != tlb.AccountStatusActive { t.Fatal("wallet not active") } - newWallet, err := mainWallet.GetSubwallet(3567745334) + newWallet, err := mainWallet.GetSubwallet(rand.Uint32()) if err != nil { t.Fatal("gen new wallet err: ", err) }