Make WalletWrite documentation for colliding accounts more precise

[str4d]

Part of #1474.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 60.82%. Comparing base (7f7b685) to head (65447c1).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1479   +/-   ##
=======================================
  Coverage   60.82%   60.82%           
=======================================
  Files         140      140           
  Lines       16472    16472           
=======================================
  Hits        10019    10019           
  Misses       6453     6453           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[daira]

Suggested change

	/// would produce the same UFVK, an error will be returned. While any combination of
	/// method calls can cause this, there are two main ways it can occur that users of the
	/// `WalletWrite` trait should watch out for:
	/// - An account is created via [`WalletWrite::create_account`] with an auto-selected
	/// ZIP 32 account index, and that index is later imported explicitly via either
	/// [`WalletWrite::import_account_hd`] or [`WalletWrite::import_account_ufvk`].
	/// - An account is imported via [`WalletWrite::import_account_ufvk`], and the ZIP 32
	/// account index corresponding to that account's UFVK is later imported either
	/// implicitly via [`WalletWrite::create_account`] or explicitly via
	/// [`WalletWrite::import_account_hd`].
	/// would produce the same UFVK, an error will be returned. This can occur in the
	/// following cases:
	/// - An account is created via [`WalletWrite::create_account`] with an auto-selected
	/// ZIP 32 account index, and that index is later imported explicitly via either
	/// [`WalletWrite::import_account_ufvk`] or [`WalletWrite::import_account_hd`].
	/// - An account is imported via [`WalletWrite::import_account_ufvk`] or
	/// [`WalletWrite::import_account_hd`], and then the ZIP 32 account index
	/// corresponding to that account's UFVK is later imported either implicitly
	/// via [`WalletWrite::create_account`], or explicitly via a call to
	/// [`WalletWrite::import_account_ufvk`] or [`WalletWrite::import_account_hd`].

This is now exhaustive, I think. I had misunderstood the semantics previously, but that just shows the importance of getting this documentation right.

[daira]

utACK

[daira]

Suggested change

	///
	///
	/// Note that accounts having UFVKs that are not identical but have shared
	/// components (for example, two accounts having the same Sapling FVK, one
	/// of which also has an Orchard FVK while the other does not) are currently
	/// allowed. This will not be allowed in future.
	///

@str4d wrote (in response to slightly different wording of the last sentence that said "may not" rather than "will not"):

NACK, I think we should decide how to handle this, and not in this PR.

Absolutely we should, but this documents the current behaviour correctly. Then in the PR where we fix it (which I agree, should be before the release), we can and should change the comment accordingly.

[daira]

These suggestions are included in #1489, which then goes on to fix it and change the comment.

[daira]

Updated my comments, and split the suggested changes for clarity.

[daira]

#1489 has been merged including these changes.