Skip to content

Merge bitcoin#19733, bitcoin#19734 #4430

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.

Sign up for GitHub

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

Jump to bottom
Merged
UdjinM6 merged 2 commits into from
Sep 24, 2021
Merged

Merge bitcoin#19733, bitcoin#19734 #4430

UdjinM6 merged 2 commits into from
Sep 24, 2021

Conversation

@vijaydasmp
Copy link

@vijaydasmp vijaydasmp commented Sep 16, 2021 •
edited
Loading

Merge bitcoin#19733: Move comment about BaseIndex::DB from TxIndex::DB …
Merge bitcoin#19734: Move CDiskTxPos to its own file

@vijaydasmp
Copy link
Author

vijaydasmp commented Sep 16, 2021 •
edited
Loading

Hello @PastaPastaPasta @UdjinM6
Getting system failure messages from CI system, somebody please check the CI system

There has been a runner system failure, please try again

@vijaydasmp vijaydasmp mentioned this pull request Sep 17, 2021
Closed
@PastaPastaPasta
Copy link
Member

PastaPastaPasta commented Sep 17, 2021

Hello @PastaPastaPasta @UdjinM6
Getting system failure messages from CI system, somebody please check the CI system

There has been a runner system failure, please try again

Have restarted those runners

UdjinM6
UdjinM6 requested changes Sep 19, 2021
View reviewed changes
src/index/base.h Outdated
Copy link

@UdjinM6 UdjinM6 Sep 18, 2021

Choose a reason for hiding this comment

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

19733: requires 19734

src/rpc/misc.cpp Outdated
Comment on lines 308 to 309
Copy link

@UdjinM6 UdjinM6 Sep 18, 2021

Choose a reason for hiding this comment

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

14667: dashify the example e.g.

                "First three receive addresses\n" +
                HelpExampleCli("deriveaddresses", "\"pkh([d34db33f/84h/0h/0h]xpub6DJ2dNUysrn5Vt36jH2KLBT2i1auw1tTSSomg8PhqNiUtx8QX2SvC9nrHu81fT41fvDUnhMjEzQgXnQjKEu3oaqMSzhSrHMxyyoEAmUHQbY/0/*)\" 0 2")

test/functional/rpc_deriveaddresses.py Outdated
Copy link

@UdjinM6 UdjinM6 Sep 18, 2021

Choose a reason for hiding this comment

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

14667: Needs dashification

src/rpc/client.cpp Outdated
Copy link

@UdjinM6 UdjinM6 Sep 18, 2021

Choose a reason for hiding this comment

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

15510: requires 15497

Copy link
Author

@vijaydasmp vijaydasmp Sep 20, 2021

Choose a reason for hiding this comment

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

Hello @UdjinM6 I tried to implement 15497 , It required Merge bitcoin#14726 which is staged with
Vlad_Staged (staged to Vladimir (dzutte-cpp) local repo) which is waiting on
Merge#14023 which is staged with you @UdjinM6 , Now we are in Blocked chain :)

src/rpc/misc.cpp Outdated
Copy link

@UdjinM6 UdjinM6 Sep 18, 2021

Choose a reason for hiding this comment

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

14150: missing changes in doc/descriptors.md, changes in src/rpc/misc.cpp do not exist in the original PR

Copy link
Author

@vijaydasmp vijaydasmp Sep 20, 2021

Choose a reason for hiding this comment

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

I observed that doc/descriptors.md this file was not present, There was a change mentioned Backport sheet for 0.17 version in 0f681ce (Add descriptor reference documentation) , however this changed was not merged , thats why this file was not present , will pull this changes

@vijaydasmp
Copy link
Author

vijaydasmp commented Sep 19, 2021

Thanks @UdjinM6 , will work on those

@vijaydasmp vijaydasmp force-pushed the backport_v18_vijay_batch4_1 branch 2 times, most recently from a97683f to 71df8f2 Compare September 20, 2021 18:27
@vijaydasmp
Copy link
Author

vijaydasmp commented Sep 21, 2021 •
edited
Loading

Below assertion is failing

[0;39;49mtest/descriptor_tests.cpp(56): �[1;31;49merror: in "descriptor_tests/descriptor_test": check parse_priv has failed�[0;39;49m
test/descriptor_tests.cpp(60): �[1;31;49merror: in "descriptor_tests/descriptor_test": check keys_priv.keys.size() has failed�[0;39;49m
Segmentation fault (core dumped)

In descriptor_test.cpp

void Check(const std::string& prv, const std::string& pub, int flags, const std::vector<std::vector<std::string>>& scripts, const std::set<std::vector<uint32_t>>& paths = ONLY_EMPTY)
{
    FlatSigningProvider keys_priv, keys_pub;
    std::set<std::vector<uint32_t>> left_paths = paths;

    // Check that parsing succeeds.
    std::unique_ptr<Descriptor> parse_priv = Parse(MaybeUseHInsteadOfApostrophy(prv), keys_priv);
    std::unique_ptr<Descriptor> parse_pub = Parse(MaybeUseHInsteadOfApostrophy(pub), keys_pub);
    BOOST_CHECK(parse_priv); **<=== This boost check is failing ,**
    BOOST_CHECK(parse_pub);

trying to understand why
@UdjinM6 @PastaPastaPasta

@vijaydasmp vijaydasmp force-pushed the backport_v18_vijay_batch4_1 branch from b6963f6 to 07fc6f8 Compare September 21, 2021 01:19
MarcoFalke and others added 2 commits September 22, 2021 23:46
@vijaydasmp
Merge bitcoin#19734: Move CDiskTxPos to its own file
a489420 
7668db3 Move only: Move CDiskTxPos to its own file (Marcin Jachymiak)

Pull request description:

  Moves `CDiskTxPos` it its own file so it can be used without the `txindex.h` include elsewhere. Originally part of bitcoin#14053.

ACKs for top commit:
  jnewbery:
    utACK 7668db3
  promag:
    ACK 7668db3.

Tree-SHA512: b108e980ad04e43d1323410c3683a82bed70aee7795f5d8a2afbaf32a07ba598571f00b047bdde15048124b17178bcbd10654c48461beac988e9643cb2df664c
@fanquake @vijaydasmp
Merge bitcoin#19733: Move comment about BaseIndex::DB from TxIndex::DB
829aeff 
8ed2f1e Remove unused includes (Marcin Jachymiak)
cf095a5 Move comment about BaseIndex::DB from TxIndex::DB (Marcin Jachymiak)

Pull request description:

  Moves a comment about the `BaseIndex::DB` from the `TxIndex::DB` into the correct place. Originally part of bitcoin#14053.

ACKs for top commit:
  fanquake:
    ACK 8ed2f1e

Tree-SHA512: cb4e2b916c7ab996961cc2e1d910bc4b8a1700eb32b70fc1657ca720117a7a84f7337fe5e4fb30e047aa92c31eaa976eaaa5cb8f861877f2ff6f4a59bb94f4e9
@vijaydasmp vijaydasmp force-pushed the backport_v18_vijay_batch4_1 branch 2 times, most recently from e907d60 to 829aeff Compare September 22, 2021 18:31
@vijaydasmp vijaydasmp changed the title Backport v18 vijay batch4 1 Merge bitcoin#19733 Merge bitcoin#19734: Sep 23, 2021
@vijaydasmp
Copy link
Author

vijaydasmp commented Sep 23, 2021

(Dropped the commits which are dependent on other commits for now) ,
Raising PR to Merge 19733 and 19734

@vijaydasmp vijaydasmp marked this pull request as ready for review September 23, 2021 02:12
@vijaydasmp vijaydasmp requested a review from UdjinM6 September 23, 2021 06:56
PastaPastaPasta
PastaPastaPasta approved these changes Sep 23, 2021
View reviewed changes
Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

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

utACK for merging via merge commit

@UdjinM6 UdjinM6 changed the title Merge bitcoin#19733 Merge bitcoin#19734: Merge bitcoin#19733, bitcoin#19734 Sep 24, 2021
UdjinM6
UdjinM6 approved these changes Sep 24, 2021
View reviewed changes
Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

utACK

@UdjinM6 UdjinM6 merged commit da26b9b into dashpay:develop Sep 24, 2021
@UdjinM6 UdjinM6 added this to the 18 milestone Sep 24, 2021
@vijaydasmp vijaydasmp deleted the backport_v18_vijay_batch4_1 branch December 29, 2021 01:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@UdjinM6 UdjinM6 UdjinM6 approved these changes

@PastaPastaPasta PastaPastaPasta PastaPastaPasta approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

18

Development

Successfully merging this pull request may close these issues.

4 participants

@vijaydasmp @PastaPastaPasta @UdjinM6 @fanquake