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.

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

[qa] test_framework: Use different rpc_auth_pair for each node #8066

Merged
merged 1 commit into from Jun 20, 2016

Conversation

Projects
None yet
4 participants
@MarcoFalke
Member

MarcoFalke commented May 17, 2016

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

@MarcoFalke MarcoFalke added the Tests label May 17, 2016

@paveljanik

This comment has been minimized.

Show comment
Hide comment
@paveljanik

paveljanik May 17, 2016

Contributor

ACK fad1845

Contributor

paveljanik commented May 17, 2016

ACK fad1845

@sipa

This comment has been minimized.

Show comment
Hide comment
@sipa

sipa May 17, 2016

Member

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

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

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

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj May 18, 2016

Member

@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?

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

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj May 18, 2016

Member

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.

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.

@MarcoFalke

This comment has been minimized.

Show comment
Hide comment
@MarcoFalke

MarcoFalke May 18, 2016

Member

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).

Member

MarcoFalke 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).

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj May 21, 2016

Member

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.

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.

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Jun 20, 2016

Member

ACK fad1845

Member

laanwj commented Jun 20, 2016

ACK fad1845

@laanwj laanwj merged commit fad1845 into bitcoin:master Jun 20, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

laanwj added a commit that referenced this pull request Jun 20, 2016

Merge #8066: [qa] test_framework: Use different rpc_auth_pair for eac…
…h node

fad1845 [qa] test_framework: Use different rpc_auth_pair for each node (MarcoFalke)

@MarcoFalke MarcoFalke deleted the MarcoFalke:Mf1605-qaAuthPairDiff branch Jun 20, 2016

@dagurval dagurval referenced this pull request Aug 29, 2017

Merged

Python 3 for qa #257

codablock added a commit to codablock/dash that referenced this pull request Sep 16, 2017

Merge bitcoin#8066: [qa] test_framework: Use different rpc_auth_pair …
…for each node

fad1845 [qa] test_framework: Use different rpc_auth_pair for each node (MarcoFalke)

codablock added a commit to codablock/dash that referenced this pull request Sep 19, 2017

Merge bitcoin#8066: [qa] test_framework: Use different rpc_auth_pair …
…for each node

fad1845 [qa] test_framework: Use different rpc_auth_pair for each node (MarcoFalke)

@schinzelh schinzelh referenced this pull request Oct 23, 2017

Closed

[WIP] Update build system to Bitcoin 0.13.2 #1692

22 of 24 tasks complete

codablock added a commit to codablock/dash that referenced this pull request Dec 27, 2017

Merge bitcoin#8066: [qa] test_framework: Use different rpc_auth_pair …
…for each node

fad1845 [qa] test_framework: Use different rpc_auth_pair for each node (MarcoFalke)

codablock added a commit to codablock/dash that referenced this pull request Dec 28, 2017

Merge bitcoin#8066: [qa] test_framework: Use different rpc_auth_pair …
…for each node

fad1845 [qa] test_framework: Use different rpc_auth_pair for each node (MarcoFalke)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment