-
Notifications
You must be signed in to change notification settings - Fork 397
feature_fedpeg.py bitcoind parent2 uses rpc cookie auth #422
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
feature_fedpeg.py bitcoind parent2 uses rpc cookie auth #422
Conversation
qa/rpc-tests/feature_fedpeg.py
Outdated
| "-deprecatedrpc=validateaddress", | ||
| # Parent chain is looking for its own conf(e.g., bitcoin.conf) | ||
| # These are not generated or cached during batch functional testing | ||
| # so we enter the required lines here |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This wording is confusing. They are needed, because the generated config files are called elements.conf and bitcoind looks for bitcoin.conf. I didn't understand that out of your explanation :laughing_smiley_with_sweat_drop:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bitcoin.conf isn't generated. What's confusing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe I misinterpreted the wording :p Still, since it's in parent_bitcoin == True, "Parent chain is looking for its own conf(e.g., bitcoin.conf)" could be "bitcoind looks for bitcoin.conf, which is not generated..." or something. But nvm, I think I just got confused with the "cached" and looking into the --cachedir for the Travis thing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i didnt feel like fixing up this comment if we end up adding more chain support in the future :)
396dc19 to
b2c2ee4
Compare
|
@stevenroose completely re-purposed this PR. |
stevenroose
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason why this is only with Bitcoin parents?
qa/rpc-tests/feature_fedpeg.py
Outdated
| self.extra_args = [] | ||
| # Parent chain args | ||
| for n in range(2): | ||
| use_cookie_auth = n==1 and self.options.parent_bitcoin |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could in principle be generalized to not only be for bitcoin, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok if we do it for that, we need to intentionally not read the elements.conf, because that would contain user/password entries.
| return rv | ||
|
|
||
| def start_node(i, dirname, extra_args=None, rpchost=None, timewait=None, binary=None, chain='elementsregtest'): | ||
| def start_node(i, dirname, extra_args=None, rpchost=None, timewait=None, binary=None, chain='elementsregtest', cookie_auth=False): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If generalized to not only be for bitcoin parents, this variable would be unnecessary since it would just be only for n==3.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you mean n==1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh yeah, thought this was about --mainchaincookiestuff, but it's about getting the proxy to connect correctly. Yeah n==1 then.
|
ok fine I'll make it work for both, since the test complexity is now causing bugs |
82f1e15 to
50fa56e
Compare
|
rpc cookie auth now happens for bitcoind or elementsd parent, by having both using a non-existent conf file. |
50fa56e to
f6528e9
Compare
|
added a spin-wait for the cookie file, since travis moves a lot slower than my local machine |
| parent_chain_signblockscript = '51' | ||
| for n in range(2): | ||
| used_cookie_auth = n==1 | ||
| rpc_u, rpc_p = rpc_auth_pair(n) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: maybe move these to lines to right before the if used_cookie_auth: line because they are only used in that block afaics.
|
Perfect! |
Resolves #420