feat: make a support of Qt app to show Platform transfer Tx#6131
feat: make a support of Qt app to show Platform transfer Tx#6131PastaPastaPasta merged 3 commits intodashpay:developfrom
Conversation
knst
added
RPC
knst
force-pushed
the
feat-withdrawals-qt
branch
2 times, most recently
from
fbd50cf to
b9118f5
Compare
UdjinM6
left a comment
UdjinM6
left a comment
There was a problem hiding this comment.
b9118f5671d2bc9947f02dbeb0c6d3331be08e4f looks incomplete - you just import a key but you don't check any tx/rpc
|
The term |
Could you suggest? just "Asset unlock" is not good too. |
PastaPastaPasta
left a comment
PastaPastaPasta
left a comment
There was a problem hiding this comment.
General NACK for v21.0; we are in feature freeze and releasing in 9 days. We need to avoid new features at all costs. We can include this in a fast following 21.1
Yes, we should think about this. I agree with @kxcd. It will be confusing because it is not a withdrawal from the wallet's perspective - it's an incoming tx just like a masternode reward, incoming payment, etc. Maybe something like |
|
Thanks Phez, I like |
knst
changed the title
knst
force-pushed
the
feat-withdrawals-qt
branch
from
b9118f5 to
70cbef9
Compare
UdjinM6
left a comment
UdjinM6
left a comment
There was a problem hiding this comment.
utACK 70cbef978ca808c74b00d378ca120317c8da7bb5 (mac-build CI failure is unrelated)
PastaPastaPasta
left a comment
PastaPastaPasta
left a comment
There was a problem hiding this comment.
general NACK on "withdrawal" terminology. Want utACK from @thephez / suggestions
current version has mentioning of |
Maybe add information about what the terms are then in description? |
src/interfaces/wallet.h
Outdated
There was a problem hiding this comment.
I know none of the others do it, but we should ensure these are always initialized
| bool is_platform_transfer; | |
| bool is_platform_transfer{false}; |
There was a problem hiding this comment.
please squash 9748a048b41a83be9703bf76d69ca46f73330d31 into a9a7f0766844543c90e249896f5ca10243c56c1d
There was a problem hiding this comment.
done in 9748a048b41...21f174aff10
knst
force-pushed
the
feat-withdrawals-qt
branch
2 times, most recently
from
3cd2115 to
9748a04
Compare
there's extra fixes and improvements. Also PR description is updated and re-made a screenshot with "platform transfer" instead "withdrawal"
rebased on the top of current develop |
thephez
left a comment
thephez
left a comment
There was a problem hiding this comment.
The "Platform transfer" terminology works for me 👍
PastaPastaPasta
force-pushed
the
feat-withdrawals-qt
branch
from
9748a04 to
21f174a
Compare
…ransfer Tx 21f174a feat: improve query categorisation in Qt App (Konstantin Akimov) c863473 test: add spending asset unlock tx in functional tests (Konstantin Akimov) 1fb67ec feat: make a support of Qt app to show Platform Transfer transaction as a new type of transaction (Konstantin Akimov) Pull request description: ## Issue being fixed or feature implemented Transfers from platform have incorrectly shown amount in Dash Core wallet app. They also shown in Qt app as self-send that is not completely true. ## What was done? Added new type of transaction to Qt App, added a filter for its type, fixed calculation of output for tx records. As well added a new type of transaction `platform-transfer` in rpc output of `gettransaction` RPC ## How Has This Been Tested? Make a Platform Transfer transaction on RegTest and check it in Dash Core  Helper to see it: export dpath=/tmp/dash_func_test_PATHPATH/ ; src/qt/dash-qt -regtest -conf=$dpath/node0/dash.conf -datadir=$dpath/node0/ -debug=0 -debuglogfile=/dev/stdout ## Breaking Changes There's new type of transaction "platform-transfer" in rpc output of `gettransaction`. **This PR DOES NOT change any consensus rules.** Breaking changes that makes withdrawal transaction immature is moved to dashpay#6128 ## Checklist: - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone Top commit has no ACKs. Tree-SHA512: ec2a54a910f121ad30ff8e94cf17080b5b3c651872e9bc3de9ec0924ca7f7a0e526b74b05cde26aaf860e3809e67f66142112319a69c216527e5bcb1b8a2b8f6
d627a6e chore: bump version to 21.1.1 (pasta) 5f9700c docs: release notes for v21.1.1 (pasta) 1c00726 Merge #6277: chore: add builder key for kittywhiskers (pasta) a2bc0f1 Merge #6290: chore: update pasta gpg key to reflect new subkeys (pasta) 167608c Merge #6338: ci: attest results of guix builds (pasta) 6fb4e49 Merge #6197: ci: always build guix, save artifacts (pasta) c0ca93c Merge #6340: fix: make 6336 compile in v21.1.x branch, using older CHECK_NONFATAL functionality (pasta) bb96df4 Merge #6336: fix: rpc getblock and getblockstats for blocks with withdrawal transactions (asset unlock) (pasta) 8e70262 Merge #6131: feat: make a support of Qt app to show Platform transfer Tx (pasta) 80ed279 Merge #6328: backport: bitcoin#30131, bitcoin#23258, bitcoin#30504 - fix bild for Ubuntu 24.10 + clang (pasta) bd772fb Merge #6229: fix: `creditOutputs` in AssetLock tx json output should be an array of objects, not debug strings (pasta) 9bf39a9 Merge #6222: fix: adjust payee predictions after mn_rr activation, add tests (pasta) 87bebfc Merge #6219: fix: correct is_snapshot_cs in VerifyDB (pasta) a4e6b8a Merge #6208: fix: persist coinjoin denoms options from gui over restarts (pasta) Pull request description: ## Issue being fixed or feature implemented See commits, backports, release notes, version bump ## What was done? ## How Has This Been Tested? ## Breaking Changes ## Checklist: _Go over all the following points, and put an `x` in all the boxes that apply._ - [ ] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: knst: utACK d627a6e kwvg: ACK d627a6e UdjinM6: utACK d627a6e ogabrielides: utACK d627a6e Tree-SHA512: cde7e40760e16e9f48da8149c3742d18a34029b057405e4d55b87110da96acbcd19b47280451dd7b5ad1ccfc91fde655452cf5f0f0d1e01a41b4c685337c64b8
36893e4 fix: add platform transfer to "most common" filter on transactions tab (Konstantin Akimov) d033a3a refactor: change mask from Dec presentation to Hex for transaction filter (Konstantin Akimov) Pull request description: ## Issue being fixed or feature implemented Follow-up #6131 - missing 'Platform Transfer' in the list of most common. Reported by splawik. ## What was done? Updated filter, added comment to prevent similar mistakes in future, present filter in hex for better readability. ## How Has This Been Tested? Transaction with platform transfer appeared in filter "Most Common"  Also they are added to Overview page (compare screenshots by 'address' field)  ## Breaking Changes N/A ## Checklist: - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone ACKs for top commit: UdjinM6: utACK 36893e4 PastaPastaPasta: utACK 36893e4 Tree-SHA512: e072b78e257b2c262a912a3cc0daebde93aca655edfee9bbf4869f2528f10377d7d234c73c4fd7ab6006e87607d5a7c4eddd7634d55b16d1b3885d0bc48f225a
36893e4 fix: add platform transfer to "most common" filter on transactions tab (Konstantin Akimov) d033a3a refactor: change mask from Dec presentation to Hex for transaction filter (Konstantin Akimov) Pull request description: ## Issue being fixed or feature implemented Follow-up dashpay#6131 - missing 'Platform Transfer' in the list of most common. Reported by splawik. ## What was done? Updated filter, added comment to prevent similar mistakes in future, present filter in hex for better readability. ## How Has This Been Tested? Transaction with platform transfer appeared in filter "Most Common"  Also they are added to Overview page (compare screenshots by 'address' field)  ## Breaking Changes N/A ## Checklist: - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone ACKs for top commit: UdjinM6: utACK 36893e4 PastaPastaPasta: utACK 36893e4 Tree-SHA512: e072b78e257b2c262a912a3cc0daebde93aca655edfee9bbf4869f2528f10377d7d234c73c4fd7ab6006e87607d5a7c4eddd7634d55b16d1b3885d0bc48f225a
5 participants
Issue being fixed or feature implemented
Transfers from platform have incorrectly shown amount in Dash Core wallet app.
They also shown in Qt app as self-send that is not completely true.
What was done?
Added new type of transaction to Qt App, added a filter for its type, fixed calculation of output for tx records.
As well added a new type of transaction
platform-transferin rpc output ofgettransactionRPCHow Has This Been Tested?
Make a Platform Transfer transaction on RegTest and check it in Dash Core
Helper to see it: export dpath=/tmp/dash_func_test_PATHPATH/ ; src/qt/dash-qt -regtest -conf=$dpath/node0/dash.conf -datadir=$dpath/node0/ -debug=0 -debuglogfile=/dev/stdout
Breaking Changes
There's new type of transaction "platform-transfer" in rpc output of
gettransaction.This PR DOES NOT change any consensus rules.
Breaking changes that makes withdrawal transaction immature is moved to #6128
Checklist: