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

Add irregular txType, add check for total balance, prevent proposal withhold attack #2587

Merged

Conversation

Projects
None yet
2 participants
@ManfredKarrer
Copy link
Member

commented Mar 26, 2019

No description provided.

ManfredKarrer added some commits Mar 26, 2019

Add IRREGULAR txType for txs which are not rule conform but not burnt
We don't want to burn BSQ in cases like that the tx was published too
late, which is a valid case if the tx does not make it in the next block.
We set such txs as IRREGULAR and allow spending of the BSQ, but there
function in the governance is invalidated.

We also add a check if the sum of all UTXO is the same as the sum of the
genesis + sum of issuance txs - burned fees.

@ManfredKarrer ManfredKarrer added this to the v0.9.6 milestone Mar 26, 2019

@ManfredKarrer ManfredKarrer requested a review from sqrrm Mar 26, 2019

Merge branch 'master' into add-check-for-bsq-balance
# Conflicts:
#	desktop/src/main/java/bisq/desktop/main/dao/wallet/dashboard/BsqDashboardView.java

@ManfredKarrer ManfredKarrer changed the title Add check for bsq balance Add irregular txType, add check for total balance Mar 26, 2019

@devinbileck devinbileck referenced this pull request Mar 26, 2019

Closed

For March 2019 #244

@sqrrm

sqrrm approved these changes Mar 26, 2019

Copy link
Member

left a comment

utACK

I have some suggested changes that you can use if you feel like.

There is also some questions on how lenient we should be with irregular txs for failed issuance, just added my thoughts.

sqrrm and others added some commits Mar 26, 2019

Update core/src/main/java/bisq/core/dao/governance/votereveal/VoteRev…
…ealService.java

Co-Authored-By: ManfredKarrer <mk@nucleo.io>
Update core/src/main/java/bisq/core/dao/governance/votereveal/VoteRev…
…ealService.java

Co-Authored-By: ManfredKarrer <mk@nucleo.io>
Update core/src/main/java/bisq/core/dao/node/parser/TxParser.java
Co-Authored-By: ManfredKarrer <mk@nucleo.io>
Update desktop/src/main/java/bisq/desktop/main/dao/wallet/dashboard/B…
…sqDashboardView.java

Co-Authored-By: ManfredKarrer <mk@nucleo.io>
@sqrrm

sqrrm approved these changes Mar 27, 2019

Copy link
Member

left a comment

utACK

ManfredKarrer and others added some commits Mar 27, 2019

Merge branch 'master' into add-check-for-bsq-balance
# Conflicts:
#	core/src/main/resources/i18n/displayStrings.properties
#	desktop/src/main/java/bisq/desktop/main/dao/wallet/dashboard/BsqDashboardView.java
Update core/src/main/java/bisq/core/dao/governance/param/Param.java
Co-Authored-By: ManfredKarrer <mk@nucleo.io>
Update core/src/main/java/bisq/core/dao/governance/param/Param.java
Co-Authored-By: ManfredKarrer <mk@nucleo.io>
Update core/src/main/java/bisq/core/dao/governance/param/Param.java
Co-Authored-By: ManfredKarrer <mk@nucleo.io>

ManfredKarrer added some commits Mar 30, 2019

Handle merits better
We did not update the merit correctly in case there was no proposal
selected.
Use burnedBsq field in Tx for burnedFee and invalidatedBsq
In case of an invalid tx we burn all available BSQ input. We only know
that at parsing time. We renamed the burntFee field to burntBsq to make
it more generic and use it for the burnt fee in case if a normal tx and
as invalidatedBsq in case of an invalid tx.
@sqrrm

sqrrm approved these changes Mar 31, 2019

Copy link
Member

left a comment

utACK

I think changing the name to burntBsq made it a lot easier to read and understand. Looks like a good solution.

ManfredKarrer added some commits Mar 31, 2019

Add more validation
- Check max length of strings and byte arrays
- Check that tx ID has 64 chars
- Add ExtraDataMapValidator for validating extraDataMap fields
Add BTC_DAO_TESTNET again to keep supporting current dao testnet
- It is more safe to separate the BTC_DAO_TESTNET and BTC_DAO_REGTEST
by the network ID as that prevents on the P2P network layer that the
network could interconnect. We would have risked that we receive network
data from the other network as users would use the persisted peers for
connections.
Merge branch 'master' into add-check-for-bsq-balance
# Conflicts:
#	core/src/main/resources/i18n/displayStrings_de.properties
#	core/src/main/resources/i18n/displayStrings_el.properties
#	core/src/main/resources/i18n/displayStrings_es.properties
#	core/src/main/resources/i18n/displayStrings_fa.properties
#	core/src/main/resources/i18n/displayStrings_fr.properties
#	core/src/main/resources/i18n/displayStrings_hu.properties
#	core/src/main/resources/i18n/displayStrings_pt.properties
#	core/src/main/resources/i18n/displayStrings_ro.properties
#	core/src/main/resources/i18n/displayStrings_ru.properties
#	core/src/main/resources/i18n/displayStrings_sr.properties
#	core/src/main/resources/i18n/displayStrings_th.properties
#	core/src/main/resources/i18n/displayStrings_vi.properties
#	core/src/main/resources/i18n/displayStrings_zh.properties
@sqrrm

sqrrm approved these changes Mar 31, 2019

Copy link
Member

left a comment

utACK

ManfredKarrer added some commits Mar 31, 2019

@ManfredKarrer ManfredKarrer marked this pull request as ready for review Mar 31, 2019

@ManfredKarrer ManfredKarrer requested a review from ripcurlx as a code owner Mar 31, 2019

@ManfredKarrer

This comment has been minimized.

Copy link
Member Author

commented Mar 31, 2019

Last commits have been rather trivial, but you are welcome to review them post merge as well.

@ManfredKarrer ManfredKarrer requested a review from cbeams as a code owner Mar 31, 2019

@ManfredKarrer

This comment has been minimized.

Copy link
Member Author

commented Mar 31, 2019

Build error is prob caused due cache issue. Local build works and witness has not changed. Pushed anotehr change to trigger re-build.

@ManfredKarrer ManfredKarrer merged commit 3854907 into bisq-network:master Mar 31, 2019

1 check failed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
@sqrrm

This comment has been minimized.

Copy link
Member

commented Apr 1, 2019

Latest changes also look good.

@ManfredKarrer ManfredKarrer deleted the ManfredKarrer:add-check-for-bsq-balance branch Apr 4, 2019

@sqrrm sqrrm referenced this pull request May 7, 2019

Open

Cycle 1 #282

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.