Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix fee output in get transaction rpc output #328

Open
wants to merge 3 commits into
base: v0.6
Choose a base branch
from

Conversation

Naviabheeman
Copy link
Contributor

@Naviabheeman Naviabheeman commented Jan 13, 2025

gettransaction rpc showed zero fees in token issue/transafer transactions. This is fixed by separating the fee calculation code and the code calculating the credit and debit of other coloured coins.
functional tests are updated to check the new location to get the fee.

token transactions were reporting incorrect fee or amount:

{
  "token": "c26de3cc6745b538738e3723f6a9a3eb297dd9ef629ef99b3b71ba251b192959d3",
  "amount": -1000,
   "fee": 0.00000000,
  "confirmations": 0,
  "trusted": true,
  "txid": "655f16523c2577584412dc52ebfb78e20b7008b0dc5273ba007d0d9015477dcb",
  "walletconflicts": [
  ],
  "time": 1735410136,
  "timereceived": 1735410136,
  "bip125-replaceable": "no",
  "details": [
    {
      "address": "vqkU6yArmetHuFkZTD6VeGzhzfagJ4y5wP3Qi6JGEBgSxdVGHxx9NGaj1DeFVeFavVMGYdedJyRcv8",
      "category": "send",
      "token": "c26de3cc6745b538738e3723f6a9a3eb297dd9ef629ef99b3b71ba251b192959d3",
      "amount": -1000,
      "vout": 0,
      "fee": -0.00000396,
      "abandoned": false
    }
  ]
  gettransaction dbe4537857ffcb0e30c6fae79a22721d3c123a30860d7488a795afdcc55e31d3
{
  "token": "TPC",
  "amount": 0.00000000,
  "fee": -0.00000258,
  "confirmations": 0,
  "trusted": true,
  "txid": "dbe4537857ffcb0e30c6fae79a22721d3c123a30860d7488a795afdcc55e31d3",
  "walletconflicts": [
  ],
  "time": 1735404354,
  "timereceived": 1735404354,
  "bip125-replaceable": "no",
  "details": [
    {
      "address": "4ZqzwozvapgvKcj4i5P3BRtXTpHmpQmkiamKaVLE6iXFeVoxm3NvGUoaHcuweD92Euyo6V1mDYLgZXc",
      "category": "receive",
      "token": "c2764880024b95656b9e7929ad685ba9f207d8115d6e2cfef3b762910aa1132fc7",
      "amount": 5000,
      "vout": 0
    }
  ]

…ther colorids in GetAmounts function.

remove code from gettransaction RPC that was ineffective
change all functional tests to look for 'fee' in the 'details' section of gettransaction rpc output
Copy link
Contributor

@azuchi azuchi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • You also need to update the manpage version.
  • Please remove 0.6.0 release note.

@@ -0,0 +1,87 @@
Tapyrus version 0.6.0 is now available for download at:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Tapyrus version 0.6.0 is now available for download at:
Tapyrus version 0.6.1 is now available for download at:

tapyrus testnet follow the instruction in [getting_started](doc/tapyrus/getting_started.md#how-to-start-a-node-on-tapyrus-testnet) to start a new Tapyrus v0.6.1 node.

If you are running a private tapyrus network using older versions of tapyrus-core release, shut it down. Wait until it has completely shut down.
Follow the instruction in [getting_started](doc/tapyrus/getting_started.md#how-to-start-a-new-tapyrus-network) to sart a new Tapyrus v0.6.0 network. Tapyrus blockchain created by older versions, before v0.5.0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Follow the instruction in [getting_started](doc/tapyrus/getting_started.md#how-to-start-a-new-tapyrus-network) to sart a new Tapyrus v0.6.0 network. Tapyrus blockchain created by older versions, before v0.5.0
Follow the instruction in [getting_started](doc/tapyrus/getting_started.md#how-to-start-a-new-tapyrus-network) to sart a new Tapyrus v0.6.1 network. Tapyrus blockchain created by older versions, before v0.5.0

@@ -1705,9 +1705,6 @@ static UniValue gettransaction(const JSONRPCRequest& request)
"2. \"include_watchonly\" (bool, optional, default=false) Whether to include watch-only addresses in balance calculation and details[]\n"
"\nResult:\n"
"{\n"
" \"amount\" : x.xxx, (numeric) The transaction amount in " + CURRENCY_UNIT + "\n"
" \"fee\": x.xxx, (numeric) The amount of the fee in " + CURRENCY_UNIT + ". This is negative and only available for the \n"
" 'send' category of transactions.\n"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was incorrect as it does not show all the tokens.
In this output the token, amount and fee are shown twice, once in the beginning and once in the details section
I removed the top one and kept the details. The RPc description was not updated correctly too, it is missing token key pair.

gettransaction dbe4537857ffcb0e30c6fae79a22721d3c123a30860d7488a795afdcc55e31d3
{
"token": "TPC",
"amount": 0.00000000,
"fee": -0.00000258,

"confirmations": 0,
"trusted": true,
"txid": "dbe4537857ffcb0e30c6fae79a22721d3c123a30860d7488a795afdcc55e31d3",
"walletconflicts": [
],
"time": 1735404354,
"timereceived": 1735404354,
"bip125-replaceable": "no",
"details": [
{
"address": "4ZqzwozvapgvKcj4i5P3BRtXTpHmpQmkiamKaVLE6iXFeVoxm3NvGUoaHcuweD92Euyo6V1mDYLgZXc",
"category": "receive",
"token": "c2764880024b95656b9e7929ad685ba9f207d8115d6e2cfef3b762910aa1132fc7",
"amount": 5000,

"vout": 0
}
]

add fee to receive address details if there was no send address from the wallet in the tx
@Naviabheeman Naviabheeman requested a review from azuchi January 15, 2025 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants