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

Support greedy addressees #106

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Support greedy addressees #106

wants to merge 2 commits into from

Conversation

jkauffman1
Copy link
Contributor

@jkauffman1 jkauffman1 commented Apr 23, 2021

Setting the 'greedy' flag on an addressee means it will consume any change instead of it going to a change output.

src/ga_tx.cpp Outdated Show resolved Hide resolved
src/ga_tx.cpp Outdated
// No need for implicit change output
continue;
}

Copy link
Contributor

Choose a reason for hiding this comment

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

At 792 we should add the dust to an explicit change output if there is one.

I think we also need a check that if any outputs are marked is_change then the tx has no added change output already.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've moved this logic around a bit so that now any dust will be added to the greedy addressee before being considered for change.

Logic should be such that there won't be a change output if there is a greedy one.

src/transaction_utils.cpp Show resolved Hide resolved
src/transaction_utils.cpp Outdated Show resolved Hide resolved
@jkauffman1 jkauffman1 force-pushed the explicit-change branch 2 times, most recently from 5f678ac to 8d74316 Compare April 23, 2021 10:37
@jkauffman1 jkauffman1 force-pushed the explicit-change branch 4 times, most recently from 5b57e2c to bf41f83 Compare May 13, 2021 21:26
@jkauffman1 jkauffman1 force-pushed the explicit-change branch 4 times, most recently from fc05354 to 1d97818 Compare May 25, 2021 11:33
@jkauffman1 jkauffman1 changed the title Support explicit change outputs Support greedy addressees Dec 2, 2021
Copy link
Contributor

@jgriffiths jgriffiths left a comment

Choose a reason for hiding this comment

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

Perhaps you want to merge the first cleanup commit, and we can look at the greedy change in isolation after review?

src/ga_tx.cpp Outdated
for (auto& addressee : *addressees_p) {
const auto addressee_asset_id = asset_id_from_json(net_params, addressee);
if (addressee_asset_id == asset_id) {
required_total += add_tx_addressee(session, net_params, result, tx, addressee);
reordered_addressees.push_back(addressee);
if (addressee.value("greedy", false)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

booleans in json should be named is_xxx, has_xxx etc.

There is no check here for multiple greedy outputs, should that error?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed to is_greedy

src/transaction_utils.cpp Outdated Show resolved Hide resolved
src/transaction_utils.hpp Outdated Show resolved Hide resolved
src/ga_tx.cpp Outdated Show resolved Hide resolved
src/ga_tx.cpp Outdated Show resolved Hide resolved
src/transaction_utils.cpp Show resolved Hide resolved
Setting the 'greedy' flag on an addressee means it will consume any
change instead of it going to a change output.
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.

None yet

2 participants