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

Complete bond domain and views #1889

Conversation

ManfredKarrer
Copy link
Member

No description provided.

Enums must not be used directly for hashCode or equals as it delivers
the Object.hashCode (internal address)!
The equals and hashCode methods cannot be overwritten in Enums.
It was only applied to classed which are used as value objects and
potentially are used in Sets or other context which involve usage of
equals or hashCode
- We store the bonded roles in the evaluated proposals in the dao state.
From there we can get the bonded role object as it is part of the
bonded role proposal. Though we need to take that data immutable
(next commit will handle that)
Use BondedRoleState to hold the mutable data. Store the BondedRoleState
in a local map.
This is the first of a series of renaming commits. We want to use role
for  the immutable class and BondedRole for the wrapper which contains
role and the mutable state.
Add ImmutableDaoStateVo interface to mark the objects used in the
daoState as immutable.
- we want to have only those classes in the daoState package which are
stored in the daoState and immutable
That was a big commit with restructuring the packages and classes.
Motivation was to isolate the daoState value objects so it is more clear
which data are help in the daoState. As it was hard to keep an overview
and easy to add mutable data I think that makes it more safe overall.
I am aware that the downside to take out domain models from the domain
packages is not so nice.
Also moved blockchain models to parser and full node packages.
That was a big commit with restructuring the packages and classes.
Motivation was to isolate the daoState value objects so it is more clear
which data are help in the daoState. As it was hard to keep an overview
and easy to add mutable data I think that makes it more safe overall.
I am aware that the downside to take out domain models from the domain
packages is not so nice.
Also moved blockchain models to parser and full node packages.
That was a big commit with restructuring the packages and classes.
Motivation was to isolate the daoState value objects so it is more clear
which data are help in the daoState. As it was hard to keep an overview
and easy to add mutable data I think that makes it more safe overall.
I am aware that the downside to take out domain models from the domain
packages is not so nice.
Also moved blockchain models to parser and full node packages.
- Add BondedRoleState to reflect all diff. states.
- Show in UI correct state and button state for lockup/revoke
- Rename ProposalService classes to ProposalFactory
- To avoid that we handle lockup/unlock 2 times in the view we will
remove the generic options (upcoming commits...)
Still WIP with many areas...
Copy link
Member

@ripcurlx ripcurlx left a comment

Choose a reason for hiding this comment

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

utACK

@ripcurlx ripcurlx merged commit f5825f6 into bisq-network:release-candidate-0.9.0 Nov 7, 2018
@ManfredKarrer ManfredKarrer deleted the complete-bonded-roles-impl branch November 7, 2018 17:20
Copy link
Member

@sqrrm sqrrm left a comment

Choose a reason for hiding this comment

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

utAck

}

public boolean isConfiscatedUnlockTxOutput(String unlockTxId) {
Copy link
Member

Choose a reason for hiding this comment

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

What is the motivation to do confiscation on both lockupTxOutput and unlockTxOutput? Wouldn't it be clearer to only use the lockupTxOutput for confiscation?

Copy link
Member Author

Choose a reason for hiding this comment

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

If the user has created already the unlock tx confiscating the lockup txo will not have any effect. For identifying we use the lockup tx.
But after a second thought, you might be right. If the lockup txo is confiscated all follow up are invalid anyway. Need to check again and see if we are filtering by unspent state...
Would need more testing anyway...

Copy link
Member

Choose a reason for hiding this comment

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

That's what I was thinking, no point in targeting the individual unlock tx since the bsq is already confiscated in the lockup. But perhaps it's needed to mark the unlock txouput as confiscated at the point of parsing the vote result if it's already being unlocked.

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

3 participants