Skip to content

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented May 8, 2020

No description provided.

@maflcko
Copy link
Member Author

maflcko commented May 8, 2020

Mostly for #14136 (and the ten other issues I closed with the same reported bug)

@luke-jr
Copy link
Member

luke-jr commented Jun 2, 2020

We would need major enhancements to support CoinJoin transactions. It's a lot more involved than just showing the correct "fee" (which this test would not check correctly anyway - some of the fee may be going to a CJ peer instead of the miner).

@maflcko
Copy link
Member Author

maflcko commented Jun 2, 2020

@luke-jr If this is unsupported, it should be documented as unsupported. See also my reply here: #19002 (comment)

@luke-jr
Copy link
Member

luke-jr commented Jun 2, 2020

Should we document every other thing that isn't supported too?

Unsupported is the default...

@DrahtBot
Copy link
Contributor

DrahtBot commented Nov 10, 2020

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK furszy
Concept ACK ismaelsadeeq

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

No conflicts as of last run.

@maflcko maflcko force-pushed the 2005-testCoinJoinGetTx branch from faf2d7d to fad82ae Compare May 9, 2022 10:25
@fanquake fanquake requested a review from instagibbs May 9, 2022 10:50
Copy link
Member

@ismaelsadeeq ismaelsadeeq left a comment

Choose a reason for hiding this comment

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

t(ACK) , I recreate and the bug exist also no need to add the node[1]'s perspective, I have tested and it's all the same for both nodes.

@maflcko maflcko force-pushed the 2005-testCoinJoinGetTx branch from fad82ae to 0000acb Compare August 23, 2023 13:36
@maflcko maflcko force-pushed the 2005-testCoinJoinGetTx branch from 0000acb to fa0da9b Compare September 14, 2023 17:01
@maflcko
Copy link
Member Author

maflcko commented Sep 15, 2023

rebased

@maflcko maflcko force-pushed the 2005-testCoinJoinGetTx branch from fa0da9b to fa20f89 Compare November 23, 2023 13:11
@fanquake fanquake requested a review from furszy November 23, 2023 14:09
Copy link
Member

@furszy furszy left a comment

Choose a reason for hiding this comment

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

utACK fa20f89

raw_hex = self.nodes[1].signrawtransactionwithwallet(raw_hex)["hex"]
txid_join = self.nodes[0].sendrawtransaction(hexstring=raw_hex, maxfeerate=0)
fee_join = self.nodes[0].getmempoolentry(txid_join)["fees"]["base"]
# Fee should be correct: assert_equal(fee_join, self.nodes[0].gettransaction(txid_join)['fee'])
Copy link
Member

@furszy furszy Nov 23, 2023

Choose a reason for hiding this comment

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

Could we add a "todo:" at the beginning?
Just to make it easier to find broken things in the future.

Copy link
Member Author

Choose a reason for hiding this comment

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

I am not a fan of TODO comments, because I think issues in the issue tracker are a better way to track outstanding feature requests and bugfixes. An issue allows anyone to leave new context, ask questions, or look up the full discussion history on the topic.

So going to leave as-is for now.

@fanquake fanquake merged commit 930bcfd into bitcoin:master Nov 23, 2023
@maflcko maflcko deleted the 2005-testCoinJoinGetTx branch November 24, 2023 09:01
@bitcoin bitcoin locked and limited conversation to collaborators Nov 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants