8000 Wallet: Add foreign_outputs metadata to support CoinJoin transactions by luke-jr · Pull Request #25991 · bitcoin/bitcoin · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Wallet: Add foreign_outputs metadata to support CoinJoin transactions #25991

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 10 commits into from
Closed

Wallet: Add foreign_outputs metadata to support CoinJoin transactions #25991

wants to merge 10 commits into from

Conversation

luke-jr
Copy link
Member
@luke-jr luke-jr commented Sep 3, 2022 •
edited
Loading

Fixes #14136

Does not fix GUI (can be done in GUI-side PR afterward)

@luke-jr luke-jr force-pushed the wallet_foreign_outputs_metadata branch 2 times, most recently from b519b35 to ae47832 Compare September 3, 2022 01:57
@DrahtBot DrahtBot added the Wallet label Sep 3, 2022
@ghost
Copy link
ghost commented Sep 3, 2022

Concept ACK.

Will test today.

Edit: It's not entirely clear at this point how to avoid older versions from opening a wallet with this metadata. Advice?

Not sure about this. If there is any error in old wallets that should work. Users with such issues can post on stackexchange, reddit etc.

Can also add information in release notes.

@DrahtBot
Copy link
Contributor
DrahtBot commented Sep 3, 2022 •
edited
Loading

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

Reviews

See the guideline for information on the review process.

Type Reviewers
Concept ACK ghost

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #27286 (wallet: Keep track of the wallet's own transaction outputs in memory by achow101)
  • #27224 (refactor: Remove CAddressBookData::destdata by ryanofsky)
  • #25979 ([WIP] wallet: standardize change output detection process by furszy)
  • #25297 (wallet: group independent db writes on single batched db transaction by furszy)
  • #24914 (wallet: Load database records in a particular order by achow101)
  • #18919 (test: Add gettransaction test for "coin-join" tx by MarcoFalke)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@ghost
Copy link
ghost commented Sep 3, 2022 •
edited by ghost
Loading

I don't think this fixes #14136

I tried it with my one of my signet wallet that was used for testing of joinstr transaction: 75e490b10b15a6a0422f25ff66ad98ef70390c8fecaac02712705dce8cc3564b

$ bitcoin-cli -rpcwallet="w5" gettransaction 75e490b10b15a6a0422f25ff66ad98ef70390c8fecaac02712705dce8cc3564b
{
  "amount": -0.00583000,
  "fee": 0.00465000,
  "confirmations": 2160,
  "blockhash": "00000007567d87fadb4673d0cb876cb1b7b9dbfedc1d9a0e40af7380d9da8435",
  "blockheight": 104271,
  "blockindex": 1,
  "blocktime": 1660978126,
  "txid": "75e490b10b15a6a0422f25ff66ad98ef70390c8fecaac02712705dce8cc3564b",
  "wtxid": "e4b91ce21961a9e5498e2ff396f5e09fa8c7f3a36ef77abd4207f71855599990",
  "walletconflicts": [
  ],
  "time": 1660976747,
  "timereceived": 1660976747,
  "bip125-replaceable": "no",
  "details": [
    {
      "address": "tb1qhxrp4zl54ul0twtyz0gury5399q7z0kvqqrl6m",
      "category": "send",
      "amount": -0.00116600,
      "vout": 0,
      "fee": 0.00465000,
      "abandoned": false
    },
    {
      "address": "tb1qnnflzn8gwadhwxr46zpxgc8fy429t25w35wah9",
      "category": "send",
      "amount": -0.00116600,
      "vout": 1,
      "fee": 0.00465000,
      "abandoned": false
    },
    {
      "address": "tb1q85ny57lu8g7na23qt7tgf03srksh9x3hrpgkvq",
      "category": "send",
      "amount": -0.00116600,
      "vout": 2,
      "fee": 0.00465000,
      "abandoned": false
    },
    {
      "address": "tb1qe36279395n6crhx3d09cptvnv72chagd336jla",
      "category": "send",
      "amount": -0.00116600,
      "vout": 3,
      "fee": 0.00465000,
      "abandoned": false
    },
    {
      "address": "tb1qkdeccjk32n0muv45xp67kq2pfd3qvfe6wym0ey",
      "category": "send",
      "amount": -0.00116600,
      "vout": 4,
      "fee": 0.00465000,
      "abandoned": false
    }
  ],
  "hex": "02000000000105b4c6a8258717bce1b92f761acf76324bb808b78879ff3ccead1452de1ad5bf0a0000000000ffffffffa8f3781e6929b82fbe8a4e821497932266a2c051b20324baba0d6ae33ffddbf00100000000ffffffff3105dc25c8bd51e921aecda35b35ca0f8ab7239f5d3452c969ce75731fcdc0f00100000000ffffffffc0c6b85c082753edfd21e9fe286f6b62dbfc6cb30d61a92667263f4057f02d170000000000fffffffff9338d880fab8b9904dbdad9c51e737b50ca670c3db9b8df195fb03e18d48cdd0100000000ffffffff0578c7010000000000160014b9861a8bf4af3ef5b96413d1c192912941e13ecc78c70100000000001600149cd3f14ce8775b771875d0826460e9255455aa8e78c70100000000001600143d264a7bfc3a3d3eaa205f9684be301da1729a3778c7010000000000160014cc74af1625a4f581dcd16bcb80ad9367958bf50d78c7010000000000160014b3738c4ad154dfbe32b43075eb01414b6206273a02473044022038884ba02e77e3c53c62412be065352b8c96b2420e117816e16b24fd6d9c47b202204257aad3ae0c9e42930ad2bb024b5adcda6f876c95b3772ae2ffa72a28327382012102449be5fb74725255eeeb50eba930fa87705f21e99d13cd710cf2c1f21153c8080247304402200a29d109643ddfb04fd26503c7e9887c9eabdc5f0ecc5072a357b91cd669ee8b022060c211a7c33eebcee308c641988e8cc67dbbe3b65856d4c69a2501a8694b0187012102131ef894942f0f9fe8f3455084c17f010549a7702bbfd74e4dff489738d0827f024730440220433cc9d973edfc3def7ec6afc49e18551e6a8af3267b75b6b72081bb80f99bbe022022473db462bcb6b698928c27fbda8bfe616cab07a62090e7dd69c4dd3a872523012103ac5d971ae3625f58c3ef5bcac46825653882bbb6a466f84d7226489dd876a1510247304402201f84bd62ab8e9986834da1f4e5e68c65d32cae7c6e2de69aa79666a53ea3696c022071a187d7248814080474e7c3a3fbbf771158f1e1c50e7d4e3c31a633f3af53fe012103025dc12c2024927ccbacb7f383d9a1a7d0709aaf73c7b22b794b82927f3be81a02473044022042636fb3f212af504b3196f27669c213342996f798aae6f50e4da0349f44c13e022076ce03586a0b6e387f330f6ef2ffc2fa952fca8c1391e0f68287ac9e17ff8381012103459928810bb656190ec36ca492f1e9c9ed84cc6ae09a0d2e9c62168bee2745b800000000"
}

@DrahtBot DrahtBot mentioned this pull request Sep 3, 2022
Closed
3 tasks
@luke-jr
Copy link
Member Author
luke-jr commented Sep 3, 2022 •
edited
Loading

@1440000bytes Did you provide it with the necessary foreign output metadata? (It's impossible to fix without that)

eg (check the outputs specified are what you intend)

bitcoin-cli -rpcwallet="w5" submitrawtransactiontowallet 02000000000105b4c6a8258717bce1b92f761acf76324bb808b78879ff3ccead1452de1ad5bf0a0000000000ffffffffa8f3781e6929b82fbe8a4e821497932266a2c051b20324baba0d6ae33ffddbf00100000000ffffffff3105dc25c8bd51e921aecda35b35ca0f8ab7239f5d3452c969ce75731fcdc0f00100000000ffffffffc0c6b85c082753edfd21e9fe286f6b62dbfc6cb30d61a92667263f4057f02d170000000000fffffffff9338d880fab8b9904dbdad9c51e737b50ca670c3db9b8df195fb03e18d48cdd0100000000ffffffff0578c7010000000000160014b9861a8bf4af3ef5b96413d1c192912941e13ecc78c70100000000001600149cd3f14ce8775b771875d0826460e9255455aa8e78c70100000000001600143d264a7bfc3a3d3eaa205f9684be301da1729a3778c7010000000000160014cc74af1625a4f581dcd16bcb80ad9367958bf50d78c7010000000000160014b3738c4ad154dfbe32b43075eb01414b6206273a02473044022038884ba02e77e3c53c62412be065352b8c96b2420e117816e16b24fd6d9c47b202204257aad3ae0c9e42930ad2bb024b5adcda6f876c95b3772ae2ffa72a28327382012102449be5fb74725255eeeb50eba930fa87705f21e99d13cd710cf2c1f21153c8080247304402200a29d109643ddfb04fd26503c7e9887c9eabdc5f0ecc5072a357b91cd669ee8b022060c211a7c33eebcee308c641988e8cc67dbbe3b65856d4c69a2501a8694b0187012102131ef894942f0f9fe8f3455084c17f010549a7702bbfd74e4dff489738d0827f024730440220433cc9d973edfc3def7ec6afc49e18551e6a8af3267b75b6b72081bb80f99bbe022022473db462bcb6b698928c27fbda8bfe616cab07a62090e7dd69c4dd3a872523012103ac5d971ae3625f58c3ef5bcac46825653882bbb6a466f84d7226489dd876a1510247304402201f84bd62ab8e9986834da1f4e5e68c65d32cae7c6e2de69aa79666a53ea3696c022071a187d7248814080474e7c3a3fbbf771158f1e1c50e7d4e3c31a633f3af53fe012103025dc12c2024927ccbacb7f383d9a1a7d0709aaf73c7b22b794b82927f3be81a02473044022042636fb3f212af504b3196f27669c213342996f798aae6f50e4da0349f44c13e022076ce03586a0b6e387f330f6ef2ffc2fa952fca8c1391e0f68287ac9e17ff8381012103459928810bb656190ec36ca492f1e9c9ed84cc6ae09a0d2e9c62168bee2745b800000000 '{"foreign_outputs": [0,1,2,3]}'

@luke-jr luke-jr force-pushed the wallet_foreign_outputs_metadata branch from ae47832 to 10bbb0a Compare September 3, 2022 17:27
@ghost
Copy link
ghost commented Sep 3, 2022

@1440000bytes Did you provide it with the necessary foreign output metadata? (It's impossible to fix without that)

eg (check the outputs specified are what you intend)

bitcoin-cli -rpcwallet="w5" submitrawtransactiontowallet 02000000000105b4c6a8258717bce1b92f761acf76324bb808b78879ff3ccead1452de1ad5bf0a0000000000ffffffffa8f3781e6929b82fbe8a4e821497932266a2c051b20324baba0d6ae33ffddbf00100000000ffffffff3105dc25c8bd51e921aecda35b35ca0f8ab7239f5d3452c969ce75731fcdc0f00100000000ffffffffc0c6b85c082753edfd21e9fe286f6b62dbfc6cb30d61a92667263f4057f02d170000000000fffffffff9338d880fab8b9904dbdad9c51e737b50ca670c3db9b8df195fb03e18d48cdd0100000000ffffffff0578c7010000000000160014b9861a8bf4af3ef5b96413d1c192912941e13ecc78c70100000000001600149cd3f14ce8775b771875d0826460e9255455aa8e78c70100000000001600143d264a7bfc3a3d3eaa205f9684be301da1729a3778c7010000000000160014cc74af1625a4f581dcd16bcb80ad9367958bf50d78c7010000000000160014b3738c4ad154dfbe32b43075eb01414b6206273a02473044022038884ba02e77e3c53c62412be065352b8c96b2420e117816e16b24fd6d9c47b202204257aad3ae0c9e42930ad2bb024b5adcda6f876c95b3772ae2ffa72a28327382012102449be5fb74725255eeeb50eba930fa87705f21e99d13cd710cf2c1f21153c8080247304402200a29d109643ddfb04fd26503c7e9887c9eabdc5f0ecc5072a357b91cd669ee8b022060c211a7c33eebcee308c641988e8cc67dbbe3b65856d4c69a2501a8694b0187012102131ef894942f0f9fe8f3455084c17f010549a7702bbfd74e4dff489738d0827f024730440220433cc9d973edfc3def7ec6afc49e18551e6a8af3267b75b6b72081bb80f99bbe022022473db462bcb6b698928c27fbda8bfe616cab07a62090e7dd69c4dd3a872523012103ac5d971ae3625f58c3ef5bcac46825653882bbb6a466f84d7226489dd876a1510247304402201f84bd62ab8e9986834da1f4e5e68c65d32cae7c6e2de69aa79666a53ea3696c022071a187d7248814080474e7c3a3fbbf771158f1e1c50e7d4e3c31a633f3af53fe012103025dc12c2024927ccbacb7f383d9a1a7d0709aaf73c7b22b794b82927f3be81a02473044022042636fb3f212af504b3196f27669c213342996f798aae6f50e4da0349f44c13e022076ce03586a0b6e387f330f6ef2ffc2fa952fca8c1391e0f68287ac9e17ff8381012103459928810bb656190ec36ca492f1e9c9ed84cc6ae09a0d2e9c62168bee2745b800000000 '{"foreign_outputs": [0,1,2,3]}'

So I need to run submitrawtransactiontowallet for every coinjoin?

@luke-jr
Copy link
Member Author
luke-jr commented Sep 3, 2022

Yes

@ghost
Copy link
ghost commented Sep 3, 2022 •
edited by ghost
Loading

Yes

This is still wrong in my case because w5 wallet did not send amount displayed in the json:

$ bitcoin-cli -rpcwallet="w5" submitrawtransactiontowallet 02000000000105b4c6a8258717bce1b92f761acf76324bb808b78879ff3ccead1452de1ad5bf0a0000000000ffffffffa8f3781e6929b82fbe8a4e821497
8000
932266a2c051b20324baba0d6ae33ffddbf00100000000ffffffff3105dc25c8bd51e921aecda35b35ca0f8ab7239f5d3452c969ce75731fcdc0f00100000000ffffffffc0c6b85c082753edfd21e9fe286f6b62dbfc6cb30d61a92667263f4057f02d170000000000fffffffff9338d880fab8b9904dbdad9c51e737b50ca670c3db9b8df195fb03e18d48cdd0100000000ffffffff0578c7010000000000160014b9861a8bf4af3ef5b96413d1c192912941e13ecc78c70100000000001600149cd3f14ce8775b771875d0826460e9255455aa8e78c70100000000001600143d264a7bfc3a3d3eaa205f9684be301da1729a3778c7010000000000160014cc74af1625a4f581dcd16bcb80ad9367958bf50d78c7010000000000160014b3738c4ad154dfbe32b43075eb01414b6206273a02473044022038884ba02e77e3c53c62412be065352b8c96b2420e117816e16b24fd6d9c47b202204257aad3ae0c9e42930ad2bb024b5adcda6f876c95b3772ae2ffa72a28327382012102449be5fb74725255eeeb50eba930fa87705f21e99d13cd710cf2c1f21153c8080247304402200a29d109643ddfb04fd26503c7e9887c9eabdc5f0ecc5072a357b91cd669ee8b022060c211a7c33eebcee308c641988e8cc67dbbe3b65856d4c69a2501a8694b0187012102131ef894942f0f9fe8f3455084c17f010549a7702bbfd74e4dff489738d0827f024730440220433cc9d973edfc3def7ec6afc49e18551e6a8af3267b75b6b72081bb80f99bbe022022473db462bcb6b698928c27fbda8bfe616cab07a62090e7dd69c4dd3a872523012103ac5d971ae3625f58c3ef5bcac46825653882bbb6a466f84d7226489dd876a1510247304402201f84bd62ab8e9986834da1f4e5e68c65d32cae7c6e2de69aa79666a53ea3696c022071a187d7248814080474e7c3a3fbbf771158f1e1c50e7d4e3c31a633f3af53fe012103025dc12c2024927ccbacb7f383d9a1a7d0709aaf73c7b22b794b82927f3be81a02473044022042636fb3f212af504b3196f27669c213342996f798aae6f50e4da0349f44c13e022076ce03586a0b6e387f330f6ef2ffc2fa952fca8c1391e0f68287ac9e17ff8381012103459928810bb656190ec36ca492f1e9c9ed84cc6ae09a0d2e9c62168bee2745b800000000 '{"foreign_outputs": [0,1,2,3]}'



$ bitcoin-cli -rpcwallet="w5" gettransaction 75e490b10b15a6a0422f25ff66ad98ef70390c8fecaac02712705dce8cc3564b
{
  "amount": -0.00583000,
  "fee": 0.00465000,
  "confirmations": 2177,
  "blockhash": "00000007567d87fadb4673d0cb876cb1b7b9dbfedc1d9a0e40af7380d9da8435",
  "blockheight": 104271,
  "blockindex": 1,
  "blocktime": 1660978126,
  "txid": "75e490b10b15a6a0422f25ff66ad98ef70390c8fecaac02712705dce8cc3564b",
  "wtxid": "e4b91ce21961a9e5498e2ff396f5e09fa8c7f3a36ef77abd4207f71855599990",
  "walletconflicts": [
  ],
  "time": 1660976747,
  "timereceived": 1660976747,
  "bip125-replaceable": "no",
  "details": [
    {
      "address": "tb1qkdeccjk32n0muv45xp67kq2pfd3qvfe6wym0ey",
      "category": "send",
      "amount": -0.00116600,
      "vout": 4,
      "fee": -0.00001400,
      "abandoned": false
    }
  ],
  "hex": "02000000000105b4c6a8258717bce1b92f761acf76324bb808b78879ff3ccead1452de1ad5bf0a0000000000ffffffffa8f3781e6929b82fbe8a4e821497932266a2c051b20324baba0d6ae33ffddbf00100000000ffffffff3105dc25c8bd51e921aecda35b35ca0f8ab7239f5d3452c969ce75731fcdc0f00100000000ffffffffc0c6b85c082753edfd21e9fe286f6b62dbfc6cb30d61a92667263f4057f02d170000000000fffffffff9338d880fab8b9904dbdad9c51e737b50ca670c3db9b8df195fb03e18d48cdd0100000000ffffffff0578c7010000000000160014b9861a8bf4af3ef5b96413d1c192912941e13ecc78c70100000000001600149cd3f14ce8775b771875d0826460e9255455aa8e78c70100000000001600143d264a7bfc3a3d3eaa205f9684be301da1729a3778c7010000000000160014cc74af1625a4f581dcd16bcb80ad9367958bf50d78c7010000000000160014b3738c4ad154dfbe32b43075eb01414b6206273a02473044022038884ba02e77e3c53c62412be065352b8c96b2420e117816e16b24fd6d9c47b202204257aad3ae0c9e42930ad2bb024b5adcda6f876c95b3772ae2ffa72a28327382012102449be5fb74725255eeeb50eba930fa87705f21e99d13cd710cf2c1f21153c8080247304402200a29d109643ddfb04fd26503c7e9887c9eabdc5f0ecc5072a357b91cd669ee8b022060c211a7c33eebcee308c641988e8cc67dbbe3b65856d4c69a2501a8694b0187012102131ef894942f0f9fe8f3455084c17f010549a7702bbfd74e4dff489738d0827f024730440220433cc9d973edfc3def7ec6afc49e18551e6a8af3267b75b6b72081bb80f99bbe022022473db462bcb6b698928c27fbda8bfe616cab07a62090e7dd69c4dd3a872523012103ac5d971ae3625f58c3ef5bcac46825653882bbb6a466f84d7226489dd876a1510247304402201f84bd62ab8e9986834da1f4e5e68c65d32cae7c6e2de69aa79666a53ea3696c022071a187d7248814080474e7c3a3fbbf771158f1e1c50e7d4e3c31a633f3af53fe012103025dc12c2024927ccbacb7f383d9a1a7d0709aaf73c7b22b794b82927f3be81a02473044022042636fb3f212af504b3196f27669c213342996f798aae6f50e4da0349f44c13e022076ce03586a0b6e387f330f6ef2ffc2fa952fca8c1391e0f68287ac9e17ff8381012103459928810bb656190ec36ca492f1e9c9ed84cc6ae09a0d2e9c62168bee2745b800000000"
}

@luke-jr
Copy link
Member Author
luke-jr commented Sep 3, 2022

Then you provided the wrong metadata, because that looks right to me?

@ghost
Copy link
ghost commented Sep 3, 2022

Then you provided the wrong metadata, because that looks right to me?

I think it was correct output but i need a break for a few days before reviewing anything

@DrahtBot DrahtBot mentioned this pull request Sep 5, 2022
Merged
amovfx
amovfx reviewed Sep 10, 2022
View reviewed changes
Copy link
@amovfx amovfx left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This was an excellent pr for the review club. I learned a ton from this.

Is this worth testing with the legacy wallet?

amovfx
amovfx reviewed Sep 12, 2022
View reviewed changes
maflcko
maflcko reviewed Sep 12, 2022
View reviewed changes
LarryRuane
LarryRuane reviewed Sep 14, 2022
View reviewed changes
@luke-jr luke-jr force-pushed the wallet_foreign_outputs_metadata branch from 0d1c407 to 1697c21 Compare September 14, 2022 18:33
stickies-v
stickies-v reviewed Sep 15, 2022
View reviewed changes
Copy link
Contributor
@stickies-v stickies-v left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Did a first code review already. If we want to track foreign outputs, this approach makes sense to me.

I'm wondering, would it be more user-friendly to instead track non-foreign outputs? For a user that is only using Core to create transactions, we could automatically track all outputs created internally and assume everything that's not in that set is foreign. This way, the user would not have to submit any additional input for listtransactions to properly work? And could still allow users to manually set outputs to be non-foreign, similar to what's implemented now? I'm not very familiar with the wallet, so apologies if I'm missing something obvious.

I think it would also be nice to update the PR description with a bit more context on what the importance of this PR is, i.e. what is the impact of the current non-sensical behaviour. Are there functional implications (I'm not sure what the general purpose of listtransactions is)?

@luke-jr
Wallet: Refactor CachedTxGetAmounts fee calculation to inline value_out
202eca1 
This effectively inline CTransaction::GetValueOut

(Bypassed MoneyRange check is not relevant here)
@luke-jr
RPC/Wallet: Add submitrawtransactiontowallet method
42c040b
@luke-jr luke-jr force-pushed the wallet_foreign_outputs_metadata branch from 1697c21 to 73c1c9c Compare September 16, 2022 22:20
@ghost
Copy link
ghost commented Sep 16, 2022 •
edited by ghost
Loading

I took a break. Although I think this PR needs to consider what is a "conjoin transaction"

@luke-jr luke-jr force-pushed the wallet_foreign_outputs_metadata branch from 73c1c9c to 9594807 Compare September 16, 2022 23:58
maflcko
maflcko reviewed Sep 17, 2022
View reviewed changes
@bitcoin bitcoin deleted a comment Sep 18, 2022
@luke-jr luke-jr requested review from maflcko, LarryRuane and stickies-v and removed request for maflcko and LarryRuane September 19, 2022 22:00
@luke-jr luke-jr force-pushed the wallet_foreign_outputs_metadata branch from bb0ab0d to da87dac Compare September 19, 2022 23:12
This was referenced Sep 20, 2022
Closed
Merged
Closed
< F438 div class="flex-auto wb-break-word" id="ref-pullrequest-1042666928" > wallet, spkm: Move key management from DescriptorScriptPubKeyMan to wallet level KeyManager #23417
Closed
This was referenced Oct 6, 2022
Closed
Closed
@DrahtBot DrahtBot mentioned this pull request Nov 13, 2022
Closed
@DrahtBot DrahtBot mentioned this pull request Jan 10, 2023
Closed
@DrahtBot DrahtBot mentioned this pull request Mar 8, 2023
Merged
@rajarshimaitra rajarshimaitra mentioned this pull request Mar 14, 2023
Closed
@DrahtBot DrahtBot mentioned this pull request Mar 21, 2023
Open
@achow101 achow101 self-requested a review April 25, 2023 15:17
@DrahtBot
Copy link
Contributor
DrahtBot commented May 1, 2023

🐙 This pull request conflicts with the target branch and needs rebase.

maflcko
maflcko reviewed May 2, 2023
View reviewed changes
src/wallet/rpc/spend.cpp
},
},
},
RPCResult{RPCResult::Type::NONE, "", ""},
Copy link
Member

Choose a reason for hiding this comment

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

For new RPCs it might be good to return an empty dict to avoid breaking changes later on and confusing behavior now.

@DrahtBot
Copy link
Contributor

There hasn't been much activity lately and the patch still needs rebase. What is the status here?

  • Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
  • Is it no longer relevant? ➡️ Please close.
  • Did the author lose interest or time to work on this? ➡️ Please close it and mark it 'Up for grabs' with the label, so that it can be picked up in the future.

@achow101
Copy link
Member

Closing as up for grabs due to lack of activity.

@achow101 achow101 closed this Sep 20, 2023
@bitcoin bitcoin locked and limited conversation to collaborators Sep 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@sipa sipa sipa left review comments

@maflcko maflcko maflcko left review comments

@LarryRuane LarryRuane LarryRuane left review comments

@amovfx amovfx amovfx left review comments

@crimscim crimscim crimscim approved these changes

@stickies-v stickies-v Awaiting requested review from stickies-v

@achow101 achow101 Awaiting requested review from achow101

Assignees
No one assigned
Labels
Needs rebase Up for grabs Wallet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

rpc: Wrong gettransaction info for a coinjoin
9 participants
@luke-jr @DrahtBot @achow101 @sipa @crimscim @maflcko @LarryRuane @amovfx @stickies-v
0