Skip to content
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

rpc: change getnewblockhex to take multiple commitments #1155

Merged
merged 2 commits into from
Sep 7, 2022

Conversation

delta1
Copy link
Member

@delta1 delta1 commented Sep 1, 2022

Modifies the getnewblockhex json rpc call to accept an array of commitments instead of a single commitment. Backwards compatibility is maintained by first parsing as a string for a single commitment.

Adds a new functional test for this rpc call.

@apoelstra
Copy link
Member

Concept ACK -- heads up you can make this a non-breaking change by explicitly accepting a string and just converting it to an array internally.

@delta1
Copy link
Member Author

delta1 commented Sep 1, 2022

@apoelstra thanks, yeah good point is that preferable?

@apoelstra
Copy link
Member

Yes, it's much preferable to keep existing RPC scripts working.

@delta1
Copy link
Member Author

delta1 commented Sep 1, 2022

Okay sure will do

@delta1 delta1 marked this pull request as draft September 1, 2022 14:57
@delta1 delta1 marked this pull request as ready for review September 2, 2022 13:15
src/rpc/mining.cpp Outdated Show resolved Hide resolved
@delta1 delta1 force-pushed the getnewblockhex-commitments branch 2 times, most recently from a2f6bd4 to ef125af Compare September 5, 2022 14:03
src/rpc/mining.cpp Outdated Show resolved Hide resolved
@stevenroose
Copy link
Member

utACK 1100877

.gitignore Outdated Show resolved Hide resolved
src/rpc/mining.cpp Outdated Show resolved Hide resolved
@apoelstra
Copy link
Member

./test/functional/rpc_help.py fails now

@delta1
Copy link
Member Author

delta1 commented Sep 6, 2022

./test/functional/rpc_help.py fails now

sorry will take a look

@apoelstra
Copy link
Member

19:11:17 -bash@camus ~/code/ElementsProject/elements/pr-review e08a86df0e5$ ./test/functional/feature_blocksign.py 
2022-09-06T19:11:19.104000Z TestFramework (INFO): Initializing test directory /tmp/bitcoin_func_test_3ghet7ze
2022-09-06T19:11:19.471000Z TestFramework (INFO): Mining and signing 101 blocks to unlock funds
2022-09-06T19:11:19.476000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
  File "/store/home/apoelstra/code/ElementsProject/elements/pr-review/test/functional/test_framework/test_framework.py", line 128, in main
    self.run_test()
  File "/store/home/apoelstra/code/ElementsProject/elements/pr-review/./test/functional/feature_blocksign.py", line 189, in run_test
    self.mine_blocks(101, False)
  File "/store/home/apoelstra/code/ElementsProject/elements/pr-review/./test/functional/feature_blocksign.py", line 178, in mine_blocks
    self.mine_block(transactions)
  File "/store/home/apoelstra/code/ElementsProject/elements/pr-review/./test/functional/feature_blocksign.py", line 128, in mine_block
    assert_equal(CScript(dummy_struct.vtx[0].vout[0].scriptPubKey).hex(), '6a04deadbeef')
  File "/store/home/apoelstra/code/ElementsProject/elements/pr-review/test/functional/test_framework/util.py", line 62, in assert_equal
    raise AssertionError("not(%s)" % " == ".join(str(arg) for arg in (thing1, thing2) + args))
AssertionError: not(51 == 6a04deadbeef)
2022-09-06T19:11:19.527000Z TestFramework (INFO): Stopping nodes
2022-09-06T19:11:19.583000Z TestFramework (WARNING): Not cleaning up dir /tmp/bitcoin_func_test_3ghet7ze
2022-09-06T19:11:19.583000Z TestFramework (ERROR): Test failed. Test logging available at /tmp/bitcoin_func_test_3ghet7ze/test_framework.log
2022-09-06T19:11:19.583000Z TestFramework (ERROR): 
2022-09-06T19:11:19.583000Z TestFramework (ERROR): Hint: Call /store/home/apoelstra/code/ElementsProject/elements/pr-review/test/functional/combine_logs.py '/tmp/bitcoin_func_test_3ghet7ze' to consolidate all logs
2022-09-06T19:11:19.583000Z TestFramework (ERROR): 
2022-09-06T19:11:19.583000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
2022-09-06T19:11:19.583000Z TestFramework (ERROR): https://github.com/ElementsProject/elements/issues
2022-09-06T19:11:19.583000Z TestFramework (ERROR): 

@delta1 delta1 force-pushed the getnewblockhex-commitments branch 2 times, most recently from cb20237 to 220de00 Compare September 7, 2022 08:49
@delta1
Copy link
Member Author

delta1 commented Sep 7, 2022

@apoelstra all fixed now

Modifies the getnewblockhex json rpc call to accept an array of
commitments instead of a single commitment.

Backwards compatibility is maintained by first attempting to parse as a
string for a singular commitment.
Copy link
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 5d10ff9

@psgreco psgreco merged commit 1cfcf1c into ElementsProject:master Sep 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants