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

For Cycle 61 #1586

Open
alvasw opened this issue Jul 30, 2024 · 1 comment
Open

For Cycle 61 #1586

alvasw opened this issue Jul 30, 2024 · 1 comment
Milestone

Comments

@alvasw
Copy link

alvasw commented Jul 30, 2024

Summary

Specify the total amount of BSQ you are requesting, along with the USD total and BSQ/USD rate (don't include the brackets!):

  • BSQ requested: 7462.69
  • USD requested: 15000
  • BSQ rate: 2.01 USD per BSQ
  • Previous compensation request (if applicable): For Cycle 60 #1571

Contributions delivered

Add contributions you have delivered and roles you have performed here as new rows in the table below. Role line-items should include an asterisk (*) in the team column.

Title Team USD Link Notes
bisq2: PpgPublicKeyParser: Warn about uncertified subkeys dev bisq-network/bisq2#2459 The Tor and the Electrum developer's subkeys are signed by their master keys. The PpgPublicKeyParser enforces this behavior. Unfortunately, this is not true for all Bitcoin Core developers. One example where this happens is when we try to verify the key with the fingerprint 0x57FF9BDBCC301009.
bisq2: build-logic: Support m out of n signature verification dev bisq-network/bisq2#2460 In Electrum all developers sign the binaries, but in Bitcoin Core a subset of its developers sign the binaries. Our existing signature verification code fails if not all developers have signed the binaries.

This change enforces that all provided signatures are signed by its developers, i.e., if we have 20 hard-coded public keys and a binary has 10 signatures we ensure that the public keys of those 10 signatures belong to the hard-coded developers and all signatures are valid.
bitcoinj: Rebase to upstream v0.16.3 dev bisq-network/bitcoinj#44 The Bisq wallet is built on top BitcoinJ, and BitcoinJ stored the total chainwork in a 12-byte field. Unfortunately, the 12-byte field was exhausted and caused BitcoinJ to fail processing blocks above chain height 849,137. The BitcoinJ developers converted the field to an unsigned 12-byte field as a hotfix 1 and we cherry-pick that change into our fork.

Later the BitcoinJ developers released a proper fix in v0.16.3 2, but our fork is based on v0.15.10. I migrated our fork to v0.16.3 3. We can't use the migrated library yet because Bisq uses removed APIs. I'm working on it.

The following commits were dropped during the rebase:
- Always use ConfidenceChange listener
If connected to 1 peer, it will be disconnected and reconnected after the broadcast, so we in fact will hear the inv from that peer.
- This commit was reverted later and this can't happen because we require connections to four peers before broadcasting a transaction and broadcast transcations simultanously over a randomly chosen mempool instance.
- Transaction: Use VarInt not opcode for script length in hashForWitnessSignature().
- Already in upstream v0.16
- Transaction, LocalTransactionSigner: Fix P2WPKH signing.
In commit bc46e85, those two occurences of scriptCode building have been missed.
- Already in upstream v0.16
- Transaction: Fix missing newline in toString().
- Already in upstream v0.16
- Update bloom filter if coins were sent
- Already in upstream v0.16 as "Wallet: When calculating the bloom filter, also include ouputs that are spent."
- Include in the filter ouputs that are spent
- Already in upstream v0.16 as "PeerGroup: Update bloom filter also if coins were sent.
- Use protobuf-gradle-plugin 0.8.10
protobuf-gradle-plugin prior to 0.8.8 have issues with Gradle 5.0+
- BitcoinJ switched in v0.16 from protobuf to protobuf-lite [?]
- https://bitcoinj.org/release-notes#version-016
- Add TransactionWitness.redeemP2WSH()
- Already in upstream v0.16
- Transaction: Add methods for calculating transaction weight and virtual transaction size.
- Already in upstream v0.16
- Transaction: Move sizes to top in toString(), and print the weight too.
- Already in upstream v0.16
- Peer: Disconnect peers that doesn't send announced transactions.
This is to prevent a memory leak of pendingTxDownloads.
- Already in upstream v0.16
- Peer: Disconnect remote peers which repeatedly don't respond to pings.
- Already in upstream v0.16
- BitcoinSerializer: Simplify if ladder in makeMessage().
- Already in upstream v0.16 as "BitcoinSerializer: Simplify if ladder in makeMessage()."
- Message: Remove never-used self check.
- Already in upstream v0.16 as "Message: Remove never-used self check."
- Script: Remove deprecated correctlySpends() variant.
- Already in upstream v0.16 as "Script: Remove deprecated correctlySpends() variant."
- Script: Deprecate non-segwit variant of correctlySpends().
- Already in upstream v0.16 as "Script: Deprecate non-segwit variant of correctlySpends().
- ScriptTest: Print failing test from tx_invalid.json.
- Already in upstream v0.16 as "ScriptTest: Print failing test from tx_invalid.json."
- Message: Fix exception due to unknown message having an unknown length. That's ok.
- Already in upstream v0.16 as "Message: Fix exception due to unknown message having an unknown length. That's ok."
- UnknownMessage: Cosmetics in toString() in case of empty payload.
- Already in upstream v0.16 as "UnknownMessage: Cosmetics in toString() in case of empty payload."
- BitcoinSerializer: Remove a redundant log message when handling an unknown message.
- Already in upstream v0.16 as "BitcoinSerializer: Remove a redundant log message when handling an unknown message."
- AddressMessage: According to https://en.bitcoin.it/wiki/Protocol_documentation#addr, it can contain only 1000 entries.
- Already in upstream v0.16 as "AddressMessage: According to https://en.bitcoin.it/wiki/Protocol_documentation#addr, it can contain only 1000 entries."
- PeerMonitor: Simplify a switch-case.
- Already in upstream v0.16 as "PeerMonitor: Simplify a switch-case."
- PeerMonitor: Increase initial size of window.
- Already in upstream v0.16 as "PeerMonitor: Increase initial size of window."
- VersionMessage: Remove support for protocol versions older than 106.
The minimum is 70000 anyway, and I'm pretty sure the code path wasn't properly tested.
- Already in upstream v0.16 as "VersionMessage: Remove support for protocol versions older than 106."
- VarInt: Introduce intValue() and longValue() accessors and use them, deprecating access to the field.
- Already in upstream v0.16 as "VarInt: Introduce intValue() and longValue() accessors and use them, deprecating access to the field."
- Message hierarchy: Use int (rather than long) for several array and string lengths.
- Already in upstream v0.16 as "Message hierarchy: Use int (rather than long) for several array and string lengths."
- Block, Transaction: Use VarInt.getSizeInBytes() on a VarInt we already have, rather than the static VarInt.sizeOf().
- Already in upstream v0.16 as "Block, Transaction: Use VarInt.getSizeInBytes() on a VarInt we already have, rather than the static VarInt.sizeOf()."
- BlockFileLoader: Remove inexact block size check.
Bitcoin Core doesn't allow over- or undersized blocks in its dat files.
- Already in upstream v0.16 as "BlockFileLoader: Remove inexact block size check."
- SendAddrV2Message: Fix 'unterminated inline tag' in a Javadoc.
- Already in upstream v0.16 as "SendAddrV2Message: Fix 'unterminated inline tag' in a Javadoc."
- UTXO: Remove Java serialization.
- Already in upstream v0.16 as "UTXO: Remove Java serialization."
- UTXO: Migrate constructor that takes a stream to a static constructor fromStream().
- Already in upstream v0.16 as "UTXO: Migrate constructor that takes a stream to a static constructor fromStream()."
- UTXO: Make fields final.
- Already in upstream v0.16 as "UTXO: Make fields final."
- MonetaryFormat: Support satoshi denomination.
- Already in upstream v0.16 as "MonetaryFormat: Support satoshi denomination."
- Exclude OP_RETURN from the MIN_ANALYSIS_NONDUST_OUTPUT check
- Already in upstream v0.14 as "TransactionOutput: New isDust() method, and use it.
- ScriptChunk: Add missing @nullable annotations to constructors.
- Already in upstream v0.16
- Make a couple of tests use the mock clock to improve test reproducibility.
- Already in upstream v0.16
- PeerGroup: Move maxOfMostFreq() from Utils to here, as here is the only usage.
- Already in upstream v0.16
- VersionMessage: Move toStringServices() from Peer to here.
- Already in upstream v0.16
- Track point compression in LazyECPoint, rather than ECPoint.
- Already in upstream v0.16
- WalletTool: Remove unused command line argument.
- Already in upstream v0.16
- AbstractBlockChain: Print error msg before throwing BlockVersionOutOfDate exception.
- From v0.14.7
- Add fileAbsolutePath in BlockStoreExeption.
- From v0.14.4
- Add fee to SendRequest
- This commit breaks the fee calculation.
- BlockingClient: Do not write to socket if it is already closed.
- This commit hides socket double close bugs.
- Changes to support connecting to .onion addresses
- Already in upstream v0.16 as "PeerAddress: Support Tor hidden service addresses.
- PeerAddress: add equalsIgnoringMetadata() method.
- We introduced this bug in a prior commit.
- PeerGroup: Add check to not duplicate peers
- We introduced this bug in a prior commit.
- PeerGroup.addInactive(): Ignore metadata fields when checking for duplicates.
- We introduced this bug in a prior commit.
- PeerGroup: Add addPeersFromAddressMessage.
- This is not needed anymore because Bisq passes a list of Bitcoin Core nodes to BitcoinJ. This functionality is implemented in upstream v0.17-alpha1 and we should cherry-pick it if we need it.
- Update harcoded seed node addresses comment
- Already in upstream v0.16 as "MainNetParams: Update hardcoded seed node addresses - Apr 2019."
- Add block store deletion / windows hack unit test
- Already in upstream v0.16 as "SPVBlockStoreTest: Disable testing SPVBlockStore deletion on Windows."
- WalletAppKit: Clear the blockchain file (as opposed to delete it) in preparation for restore.
This adds a new clear() method to SPVBlockStore.
- Already in upstream v0.16
- Wallet: Implement witness fee discount.
Fee is now specified in virtual (kilo)bytes. For non-segwit transactions a virtual byte is the same as a byte so the change is backward compatible.
- Already in upstream v0.16
- Use FilterRecalculateMode.SEND_IF_CHANGED
- Reverted in Revert "Use FilterRecalculateMode.SEND_IF_CHANGED"
- Readd return statement removed by mistake
- Part of reverted commit "Use FilterRecalculateMode.SEND_IF_CHANGED"
- Update bloom filter if coins were sent
- Already in upstream v0.16 as "Wallet: When calculating the bloom filter, also include ouputs that are spent." and "PeerGroup: Update bloom filter also if coins were sent."
- AlertMessage: Remove alert messages.
The alert message facility has been removed from the Bitcoin protocol due to its centralized nature.
- Already in upstream v0.16
- Tighten two try blocks when sending.
- Already in upstream v0.16
- MessageSerializer: Move protocolVersion from Message to here.
This change moves protocolVersion from Message to MessageSerializer.
The main reason is to support parsing transactions as non-segwit.
This is impossible to do otherwise, as MessageSerializer#makeTransaction()
does not accept a protocolVersion parameter.
Additionally this change makes the MessageSerializer similar to the
Stream in Bitcoin Core, where the protocol version can be modified.
- Already in upstream v0.16
- Message hierarchy: Remove constructors that take a separate protocolVersion argument.
It's contained in the MessageSerializer since a while.
- Already in upstream v0.16
- Message: Make readVarInt() return a VarInt rather than long.
- Already in upstream v0.16
- Support BIP155 addrv2 messages.
- Already in upstream v0.16
- PeerAddress: Support Tor hidden service addresses.
- Already in upstream v0.16
- Support BIP133 feefilter messages.
- Already in upstream v0.16
- UTXO: Make index, hash and value the identity.
- Already in upstream v0.16
- LazyECPoint: JavaDoc for the constructors.
- Already in upstream v0.16
- Github Actions: Build with Gradle 6.9
Use the gradle/gradle-build-action to build with a specified
version of Gradle. In our case, let’s use Gradle 6.9.
- Already in upstream v0.16
- README.md: Reflect max Gradle of 6.9.
- Already in upstream v0.16
- Add deprecated constructors
- Unsupported in upstream version now
- Add Tor v3 onion address support and minor code clean up
- The implementation is wrong.
- Remove dead code parts, improve documentation and fix broken unit tests
- Part of Add Tor v3 onion address support and minor code clean up
- Reduce log level of two messages which frequently fill user's log.
Bisq can get into a state where it checks for network peers
constantly, filling the logs with the connection attempts. e.g.
Peer discovery didn't provide us any more peers, will try again in
10000ms According to the log message, there should be a 10000ms delay
between connection attempts, but examples clearly contradict this.
The problem with these messages spamming all 20 logfiles is that it
prevents other, valid trade issues from being investigated. A high
percentage of supplied user logs contain examples of this.
I'm hoping this can be fixed in Bisq's fork of BitcoinJ.
- Our peer discovery was broken and caused this bug.
dev 15000 Total for items above.

Footnotes

  1. StoredBlock: use Utils.bigIntegerToBytes to convert chainWork (Backport)

  2. Version 0.16.3 - Release Notes

  3. https://github.com/alvasw/bitcoinj/tree/bisq_0.16.3 |

@MwithM MwithM added this to the Cycle 61 milestone Jul 30, 2024
@alvasw
Copy link
Author

alvasw commented Aug 6, 2024

b9aaa94b2626da29adace567c59318c6b402c4d95f3817b55eb7d445074af06f

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Proposal Submitted
Development

No branches or pull requests

2 participants