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.

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

HTLC Changes #1998

Merged
merged 31 commits into from
Apr 28, 2020
Merged

HTLC Changes #1998

merged 31 commits into from
Apr 28, 2020

Conversation

jmjatlanta
Copy link
Contributor

@jmjatlanta jmjatlanta commented Sep 20, 2019 •
edited
Loading

Implementation of BSIP 64

NOTE: bitshares/bitshares-fc#143 must be merged and FC bumped for HASH160 changes

@jmjatlanta jmjatlanta mentioned this pull request Apr 7, 2020
Closed
@jmjatlanta jmjatlanta marked this pull request as ready for review April 8, 2020 18:13
abitmore
abitmore reviewed Apr 13, 2020
View reviewed changes
libraries/protocol/include/graphene/protocol/htlc.hpp Show resolved Hide resolved
libraries/wallet/operation_printer.cpp Outdated Show resolved Hide resolved
tests/tests/htlc_tests.cpp Show resolved Hide resolved
libraries/chain/htlc_evaluator.cpp Outdated Show resolved Hide resolved
tests/tests/htlc_tests.cpp Outdated Show resolved Hide resolved
libraries/wallet/wallet.cpp Show resolved Hide resolved
tests/tests/htlc_tests.cpp Outdated Show resolved Hide resolved
@abitmore
Copy link
Member

The code doesn't compile. Seems we need to bump FC.

@jmjatlanta
Copy link
Contributor Author

The code doesn't compile. Seems we need to bump FC.

FC PR ready for your review. Once merged, FC can be bumped in this branch.

@abitmore
Copy link
Member

bitshares/bitshares-fc#143 has been merged. Please bump FC. Thanks.

@jmjatlanta jmjatlanta requested a review from abitmore April 20, 2020 16:14
abitmore
abitmore reviewed Apr 20, 2020
View reviewed changes
tests/tests/htlc_tests.cpp Outdated Show resolved Hide resolved
libraries/protocol/fee_schedule_calc.cpp Outdated Show resolved Hide resolved
@abitmore
Copy link
Member

Travis build still fails.

abitmore
abitmore reviewed Apr 23, 2020
View reviewed changes
Copy link
Member

@abitmore abitmore left a comment

Choose a reason for hiding this comment

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

The fee_schedule code refactory is not a priority, that's why I created issue #2150 and set its milestone to 4.1.

libraries/protocol/include/graphene/protocol/fee_schedule.hpp Outdated Show resolved Hide resolved
libraries/protocol/include/graphene/protocol/fee_schedule.hpp Outdated Show resolved Hide resolved
@jmjatlanta jmjatlanta mentioned this pull request Apr 23, 2020
Draft
1 task
abitmore
abitmore reviewed Apr 23, 2020
View reviewed changes
Copy link
Member

@abitmore abitmore left a comment

Choose a reason for hiding this comment

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

It seems more work is needed. Thanks anyway.

libraries/chain/proposal_evaluator.cpp Outdated Show resolved Hide resolved
libraries/chain/proposal_evaluator.cpp Outdated Show resolved Hide resolved
libraries/protocol/fee_schedule_calc.cpp Show resolved Hide resolved
libraries/protocol/include/graphene/protocol/htlc.hpp Show resolved Hide resolved
tests/tests/htlc_tests.cpp Outdated Show resolved Hide resolved
abitmore
abitmore reviewed Apr 26, 2020
View reviewed changes
tests/tests/htlc_tests.cpp Outdated Show resolved Hide resolved
tests/tests/htlc_tests.cpp Outdated Show resolved Hide resolved
tests/tests/htlc_tests.cpp Outdated Show resolved Hide resolved
tests/tests/htlc_tests.cpp Outdated Show resolved Hide resolved
libraries/protocol/include/graphene/protocol/htlc.hpp Show resolved Hide resolved
abitmore
abitmore reviewed Apr 26, 2020
View reviewed changes
libraries/chain/proposal_evaluator.cpp Outdated Show resolved Hide resolved
tests/cli/main.cpp Show resolved Hide resolved
tests/cli/main.cpp Outdated Show resolved Hide resolved
tests/cli/main.cpp Outdated Show resolved Hide resolved
tests/tests/htlc_tests.cpp Outdated Show resolved Hide resolved
tests/tests/htlc_tests.cpp Outdated Show resolved Hide resolved
tests/tests/htlc_tests.cpp Outdated Show resolved Hide resolved
tests/tests/htlc_tests.cpp Outdated Show resolved Hide resolved
tests/tests/htlc_tests.cpp Outdated Show resolved Hide resolved
tests/tests/htlc_tests.cpp Outdated Show resolved Hide resolved
abitmore
abitmore reviewed Apr 26, 2020
View reviewed changes
tests/tests/htlc_tests.cpp Show resolved Hide resolved
tests/tests/htlc_tests.cpp Outdated Show resolved Hide resolved
abitmore
abitmore approved these changes Apr 27, 2020
View reviewed changes
Copy link
Member

@abitmore abitmore left a comment

Choose a reason for hiding this comment

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

The code looks fine (finally), although there are some long lines.
Thanks for the efforts.
I'm now approving, but still waiting for the CI results.

@abitmore abitmore merged commit bb8493d into hardfork Apr 28, 2020
@abitmore abitmore mentioned this pull request Apr 28, 2020
Closed
@abitmore
Copy link
Member

abitmore commented Apr 28, 2020 •
edited
Loading

Merged too early. Found a new issue: the preimage doesn't get stored into the preimage field added in htlc_redeemed_operation.

@jmjatlanta
Copy link
Contributor Author

jmjatlanta commented Apr 28, 2020 •
edited
Loading

Not a big problem. I will fix right away in a subsequent PR, and fix the line lengths.

@abitmore abitmore deleted the jmj_bsip_64 branch April 28, 2020 20:27
@MichelSantos MichelSantos mentioned this pull request May 12, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@abitmore abitmore abitmore approved these changes

Assignees
No one assigned
Labels
hardfork
Projects
None yet
Milestone
4.0.0 - Protocol Upgrade Release
Development

Successfully merging this pull request may close these issues.
3 participants
@jmjatlanta @abitmore @pmconrad