Skip to content

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented May 17, 2016

This gets rid of the hardcoded user='rt', pass='rt'.

@maflcko maflcko added the Tests label May 17, 2016
@paveljanik
Copy link
Contributor

ACK fad1845

@sipa
Copy link
Member

sipa commented May 17, 2016

Is there a benefit to having different rpcauth pairs for each node?

(that's a separate question from not hardcoding it)

@laanwj
Copy link
Member

laanwj commented May 18, 2016

@sipa I think the motivation is that it prevents accidentally connecting to the wrong/old instance which may still be sticking around.
Speaking of which, should we add a random per-session factor too?

@laanwj
Copy link
Member

laanwj commented May 18, 2016

But one drawback I can see of this is that it makes it more difficult to diagnose after a non-cleanup test run, or interrogating a bitcoind stuck around after a crashing test. No longer is the user/password to query it obvious.

@maflcko maflcko force-pushed the Mf1605-qaAuthPairDiff branch from fad1845 to fa6e082 Compare May 18, 2016 15:28
@maflcko
Copy link
Member Author

maflcko commented May 18, 2016

Indeed this is the drawback, which would mean I better remove the fancy utf-8 chars as well...

Obviously we are running the daemons on different ports, so within the test framework it is more than unlikely to accidentally speak to the wrong daemon. I think a reasonable compromise would be to use 'rt_u'+str(n) and 'rt_p'+str(n).

@maflcko maflcko force-pushed the Mf1605-qaAuthPairDiff branch from b3ab412 to fa6e082 Compare May 21, 2016 05:13
@laanwj
Copy link
Member

laanwj commented May 21, 2016

No longer is the user/password to query it obvious.

I just realized this may be a smaller problem than I thought: we write a bitcoin.conf in every node directory with the user/password information so bitcoin-cli -datadir=$DIR can be used. As the directory is printed to stdout, I think there is not really a problem with 'guessing' the user/password.

@maflcko maflcko force-pushed the Mf1605-qaAuthPairDiff branch from fa6e082 to fad1845 Compare May 21, 2016 14:32
@maflcko maflcko force-pushed the Mf1605-qaAuthPairDiff branch from bbe9f58 to fad1845 Compare June 3, 2016 11:40
@laanwj
Copy link
Member

laanwj commented Jun 20, 2016

ACK fad1845

@laanwj laanwj merged commit fad1845 into bitcoin:master Jun 20, 2016
laanwj added a commit that referenced this pull request Jun 20, 2016
…h node

fad1845 [qa] test_framework: Use different rpc_auth_pair for each node (MarcoFalke)
@maflcko maflcko deleted the Mf1605-qaAuthPairDiff branch June 20, 2016 11:42
codablock pushed a commit to codablock/dash that referenced this pull request Dec 28, 2017
…for each node

fad1845 [qa] test_framework: Use different rpc_auth_pair for each node (MarcoFalke)
andvgal pushed a commit to energicryptocurrency/gen2-energi that referenced this pull request Jan 6, 2019
…for each node

fad1845 [qa] test_framework: Use different rpc_auth_pair for each node (MarcoFalke)
zkbot added a commit to zcash/zcash that referenced this pull request Dec 2, 2020
Backport migration from rpc-tests.sh to rpc-tests.py

Cherry-picked from the following upstream PRs:
- bitcoin/bitcoin#6567
- bitcoin/bitcoin#6523
- bitcoin/bitcoin#6616
- bitcoin/bitcoin#6788
  - Only the commit fixing `rpc-tests.py`
- bitcoin/bitcoin#6791
  - Only the fix to `qa/rpc-tests/README.md`
- bitcoin/bitcoin#6827
- bitcoin/bitcoin#6930
- bitcoin/bitcoin#6804
- bitcoin/bitcoin#7029
- bitcoin/bitcoin#7028
- bitcoin/bitcoin#7027
- bitcoin/bitcoin#7135
- bitcoin/bitcoin#7209
- bitcoin/bitcoin#7635
- bitcoin/bitcoin#7778
- bitcoin/bitcoin#7851
- bitcoin/bitcoin#7814
  - Only the changes to the new .py files in this PR.
- bitcoin/bitcoin#7971
- bitcoin/bitcoin#7972
- bitcoin/bitcoin#8056
  - Only the first commit.
- bitcoin/bitcoin#8098
- bitcoin/bitcoin#8104
- bitcoin/bitcoin#8133
  - Only the `rpc-tests.py` commit.
- bitcoin/bitcoin#8066
- bitcoin/bitcoin#8216
  - Only the last two commits.
- bitcoin/bitcoin#8254
- bitcoin/bitcoin#8400
- bitcoin/bitcoin#8482
  - Excluding the first commit (only affects RPC tests we don't have).
- bitcoin/bitcoin#8551
- bitcoin/bitcoin#8607
  - Only the pull-tester commit, for conflict removal.
- bitcoin/bitcoin#8625
- bitcoin/bitcoin#8713
- bitcoin/bitcoin#8750
- bitcoin/bitcoin#8789
- bitcoin/bitcoin#9098
- bitcoin/bitcoin#9276
  - Excluding the second commit (we don't have the changes it requires).
- bitcoin/bitcoin#9657
- bitcoin/bitcoin#9807
- bitcoin/bitcoin#9766
- bitcoin/bitcoin#9823
zkbot added a commit to zcash/zcash that referenced this pull request Dec 2, 2020
Backport migration from rpc-tests.sh to rpc-tests.py

Cherry-picked from the following upstream PRs:
- bitcoin/bitcoin#6567
- bitcoin/bitcoin#6523
- bitcoin/bitcoin#6616
- bitcoin/bitcoin#6788
  - Only the commit fixing `rpc-tests.py`
- bitcoin/bitcoin#6791
  - Only the fix to `qa/rpc-tests/README.md`
- bitcoin/bitcoin#6827
- bitcoin/bitcoin#6930
- bitcoin/bitcoin#6804
- bitcoin/bitcoin#7029
- bitcoin/bitcoin#7028
- bitcoin/bitcoin#7027
- bitcoin/bitcoin#7135
- bitcoin/bitcoin#7209
- bitcoin/bitcoin#7635
- bitcoin/bitcoin#7778
- bitcoin/bitcoin#7851
- bitcoin/bitcoin#7814
  - Only the changes to the new .py files in this PR.
- bitcoin/bitcoin#7971
- bitcoin/bitcoin#7972
- bitcoin/bitcoin#8056
  - Only the first commit.
- bitcoin/bitcoin#8098
- bitcoin/bitcoin#8104
- bitcoin/bitcoin#8133
  - Only the `rpc-tests.py` commit.
- bitcoin/bitcoin#8066
- bitcoin/bitcoin#8216
  - Only the last two commits.
- bitcoin/bitcoin#8254
- bitcoin/bitcoin#8400
- bitcoin/bitcoin#8482
  - Excluding the first commit (only affects RPC tests we don't have).
- bitcoin/bitcoin#8551
- bitcoin/bitcoin#8607
  - Only the pull-tester commit, for conflict removal.
- bitcoin/bitcoin#8625
- bitcoin/bitcoin#8713
- bitcoin/bitcoin#8750
- bitcoin/bitcoin#8789
- bitcoin/bitcoin#9098
- bitcoin/bitcoin#9276
  - Excluding the second commit (we don't have the changes it requires).
- bitcoin/bitcoin#9657
- bitcoin/bitcoin#9807
- bitcoin/bitcoin#9766
- bitcoin/bitcoin#9823
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
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.

4 participants