accounts: improved utxosToAccount

[kuegi]

Summary

when you create a utxostoaccount message, the wallet selects "random" utxos to use. Most of the time such a tx is used to transfer utxos from the address to the token layer of the same address. The random selection therefore just messes with the accounting (pull utxos from other addresses in the wallet).

This PR changes that: if there is only 1 target address, and the target address is owned by the account, only utxos of that address are used (if enough are available)

RPCs

changes in input selection of utxostoaccount

Implications

• Storage

  • Database reindex required
  • Database reindex optional
  • Database reindex not required
  • None

• Consensus

  • Network upgrade required
  • Includes backward compatible changes
  • Includes consensus workarounds
  • Includes consensus refactors
  • None

[prasannavl]

Thanks @kuegi. This is a neat low hanging fruit find.

Let's see if @Bushstar has any thoughts on this. It does create an ad-hoc branch, perhaps we can merge it into fund by merging it into the coin select strategy if we can apply this to many other places safely (and passing down the target as hints)

[prasannavl]

If it's too much effort to reuse, this looks good to merge to me.

• Would appreciate if you could clean up the lints @kuegi. ./make.sh fmt should take care of it.
• Test failures are just the known flaky ones, can be ignored.

[kuegi]

Let's see if @Bushstar has any thoughts on this. It does create an ad-hoc branch, perhaps we can merge it into fund by merging it into the coin select strategy if we can apply this to many other places safely (and passing down the target as hints)

honestly no idea what an ad-hoc branch is in this context. Was thinking about adapting fund but I am lacking insights there so I didn't want to break anything.

basically all other places require an auth-input from the "owner" of the customTx which makes it use one utxo from there and also forces the change address. AFAIK utxos2account was the only one left, as it does not need an auth-input.
But merging the logic into fund would clearly be the best option for general code quality.

[kuegi]

If it's too much effort to reuse, this looks good to merge to me.

• Would appreciate if you could clean up the lints @kuegi. ./make.sh fmt should take care of it.
• Test failures are just the known flaky ones, can be ignored.

I ran make.sh fmt but just now realized that it is missing clang-format(-15) and therefore didn't format the cpp part. will fix.

[kuegi]

If it's too much effort to reuse, this looks good to merge to me.

• Would appreciate if you could clean up the lints @kuegi. ./make.sh fmt should take care of it.
• Test failures are just the known flaky ones, can be ignored.

with clang-format installed, fmt gives me a lot of additional changes:

Is there some setting that you have differently?

[Bushstar]

with clang-format installed, fmt gives me a lot of additional changes: Is there some setting that you have differently?

Maybe double check the version is 15 as just installed clang-format can end up with a different version.

[kuegi]

Maybe double check the version is 15 as just installed clang-format can end up with a different version.

true, I got version 18 installed. need to find a way to install 15 (homebrew only has 8 and 11)...

[kuegi]

with clang-format installed, fmt gives me a lot of additional changes: Is there some setting that you have differently?

Maybe double check the version is 15 as just installed clang-format can end up with a different version.

according to the .clang-format, QualifierAlignment: Leftis set, which would mean that its const autoand not auto const , but there are many places in the code with auto const so it seems to me that not all code got clang-format applied yet?

[prasannavl]

Is there some setting that you have differently?

./make.sh fmt should use the right config automatically, so odd if it resulted in a different output. Possibly either an overenthusiastic editor formatted with different config or may be a different version of clang fmt that made different assumptions.

according to the .clang-format, QualifierAlignment: Leftis set, which would mean that its const autoand not auto const , but there are many places in the code with auto const so it seems to me that not all code got clang-format applied yet?

There are a bunch of clang format configs. Unfortunate inheritance from Bitcoin. They also never seem to have applied the configs properly, so the codebase isn't coherent and we can't reformat these as well as is makes merging or patching from upstream impossible after reformatting.

What we do to find middle ground is we enforce our config for everything under the src/dfi/ or code that we commit in. Rest, we follow the existing conventions where is there is.

[prasannavl]

The specific ruleset we do is here: https://github.com/DeFiCh/ain/blob/master/make.sh/#L489-L519

• src/dfi
• src/ffi
• Then a few whitelist that we know is safe to reformat which plugs into existing code that we've extracted out over time.

[prasannavl]

Edit: Please ignore. Local error.

Likely caused bug: Failing on sign step after this PR on master on bech32 addresses.

Can't sign TX: [{"txid":"<..>","vout":1,"witness":[],"scriptSig":"","sequence":4294967294,"error":"Witness program hash mismatch"},{"txid":"<..>","vout":1,"witness":[],"scriptSig":"","sequence":4294967294,"error":"Witness program hash mismatch"}]

[kuegi]

Edit: Please ignore. Local error.

Likely caused bug: Failing on sign step after this PR on master on bech32 addresses.

Can't sign TX: [{"txid":"<..>","vout":1,"witness":[],"scriptSig":"","sequence":4294967294,"error":"Witness program hash mismatch"},{"txid":"<..>","vout":1,"witness":[],"scriptSig":"","sequence":4294967294,"error":"Witness program hash mismatch"}]

anyway, I added tests for the behaviour: #2963