Fix non-deterministic behaviour for custom accounts #7565
Conversation
| // newAccountNotFoundError returns an error indicating that the manager didn't | ||
| // find the specific account. This error is used to be compatible with the old | ||
| // 'LookupAccount' behaviour previously used. | ||
| func newAccountNotFoundError(name string) error { |
There was a problem hiding this comment.
I used this particular error to be compatible with the old error returned by LookupAccount in the same case.
If this compatibility is not important, I will create a generic error like
var errAccountNotFound
Torakushi
force-pushed
the
accounts
branch
5 times, most recently
from
0fb34fb
to
8a7a62b
Compare
|
it looks like that a 30m timeout was added to the travis build: Making the travis build failed (timeout) I've seen this behaviour in other PRs (example: https://app.travis-ci.com/github/lightningnetwork/lnd/builds/261768324) |
My pleasure 👍 sorry it took so long!
Thank you @guggero! |
|
@sputn1ck: review reminder |
|
Don't really know why a test is failing after the rebase As well Travis job is failing (weird error), I will check tomorrow! |
|
Looks like that failure is independent. Some tests unfortunately are still flaky. |
|
Indeed flaky tests :) |
In theory, it should be only one custom account with a given name. However, due to a lack of check, users could have created custom accounts with various key scopes. In that case, ListAccounts has to list all these accounts.
In theory, it should be only one custom account with a given name. However, users could have created custom accounts with various key scopes. In that case, 'LookupAccount' has a non deterministic behaviour. To fix that, we browse through all key scopes (deterministically) and return the first account we found.
Currently, a user can create a custom account with various key scopes. This is not a desired behaviour.
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
|
Concept Ack |
This PR fixes: #7265
fix non-deterministic behaviour for custom accounts with same name but different key scopes (ListAccounts/LookupAccount)
Forbid users to create a custom account if the name is already used
Steps to Test
This PR was tested, here are the steps to test:
-> You will need to cherry-pick the two first commit (otherwise the 3rd commit will prevent you from create 2 custom accounts with the same name). Then import 2 accounts (different key scope but same name) and check if
ListAccountsreturns the 2 accounts (and not only one as before)-> Now, you can include the 3rd commit, check that you cant import 2 accounts with the same name anymore