Skip to content

Conversation

sipa
Copy link
Member

@sipa sipa commented Apr 14, 2016

This is a preparation commit for segwit, but won't hurt to include ahead of time.

It is necessary as the relayed serialization will depend on what the peer requests.

@btcdrak
Copy link
Contributor

btcdrak commented Apr 14, 2016

@sipa Should this also be marked as backport for 0.12?

@laanwj
Copy link
Member

laanwj commented Apr 14, 2016

I think with segwit we're getting at the point we're going to need so many backports for 0.12.2 we'd better just release 0.13 early...

utACK 38c3102

@maflcko
Copy link
Member

maflcko commented Apr 14, 2016

I'd agree with backporting this "early" (as soon as it is merged into master) to avoid confusion and cherry-pick conflicts due to "out-of-order" cherry-picks, but it seems this is already conflicting on 0.12, so better leave such conflicting cherry-picks to a separate pull request with review before merge.

@sipa
Copy link
Member Author

sipa commented Apr 14, 2016

@MarcoFalke My segwit-base branch is 0.12-based, and includes backports from master and other pull requests needed already.

@laanwj
Copy link
Member

laanwj commented Apr 14, 2016

I'd agree with backporting this "early" (as soon as it is merged into master)

That would rule out merging this for now - there is a release in progress on the 0.12 branch, so only things that are critical enough to warrant a rc3 should be merged there right now.

But I'd prefer to backport this as part of other segwit changes, also to have the context clear.

@sdaftuar
Copy link
Member

ACK 38c3102

In case anyone else has the same thought: I was wondering what the memory usage ratio was between a CTransaction and its serialization; it seems to be about 1.4x as big, which is small enough that it should be better to store the CTransaction than both of the serializations that would generally be needed post-segwit. (Also serialization seems to be fast enough that it's not worth worrying about doing it for each peer.)

@jonasschnelli
Copy link
Contributor

utACK 38c3102

@theuni
Copy link
Member

theuni commented Apr 15, 2016

nice, I had the same change locally for a different reason. Freebie :)

utACK.

@laanwj
Copy link
Member

laanwj commented Apr 15, 2016

I was wondering what the memory usage ratio was between a CTransaction and its serialization; it seems to be about 1.4x as big, which is small enough that it should be better to store the CTransaction than both of the serializations that would generally be needed post-segwit. (Also serialization seems to be fast enough that it's not worth worrying about doing it for each peer.)

Thanks for checking this, it was in the back of my mind as well.

@laanwj laanwj merged commit 38c3102 into bitcoin:master Apr 15, 2016
laanwj added a commit that referenced this pull request Apr 15, 2016
38c3102 Change mapRelay to store CTransactions (Pieter Wuille)
codablock pushed a commit to codablock/dash that referenced this pull request Dec 22, 2017
38c3102 Change mapRelay to store CTransactions (Pieter Wuille)
codablock added a commit to codablock/dash that referenced this pull request Dec 22, 2017
Bitcoin bitcoin#7877 changed mapRelay to be indexed by hash instead of inv. This
means that we may end up with a false-positive match here and send out an
instant transaction as normal transaction.
andvgal pushed a commit to energicryptocurrency/gen2-energi that referenced this pull request Jan 6, 2019
38c3102 Change mapRelay to store CTransactions (Pieter Wuille)
andvgal pushed a commit to energicryptocurrency/gen2-energi that referenced this pull request Jan 6, 2019
Bitcoin bitcoin#7877 changed mapRelay to be indexed by hash instead of inv. This
means that we may end up with a false-positive match here and send out an
instant transaction as normal transaction.
zkbot added a commit to zcash/zcash that referenced this pull request Aug 4, 2021
ZIP 239 preparations 1

This is the first of several backports to prepare for ZIP 239. The primary
change is altering `mapRelay` to store `CTransaction`s, which we need
because ZIP 239 requires changing `Inv` messages based on transaction
versions. The other changes are mainly for conflict removal but are also
independently useful.

Backports the following upstream PRs:
- bitcoin/bitcoin#6889
- bitcoin/bitcoin#7125
- bitcoin/bitcoin#7862
- bitcoin/bitcoin#7877
zkbot added a commit to zcash/zcash that referenced this pull request Aug 5, 2021
ZIP 239 preparations 1

This is the first of several backports to prepare for ZIP 239. The primary
change is altering `mapRelay` to store `CTransaction`s, which we need
because ZIP 239 requires changing `Inv` messages based on transaction
versions. The other changes are mainly for conflict removal but are also
independently useful.

Backports the following upstream PRs:
- bitcoin/bitcoin#6889
- bitcoin/bitcoin#7125
- bitcoin/bitcoin#7862
- bitcoin/bitcoin#7877
zkbot added a commit to zcash/zcash that referenced this pull request Aug 10, 2021
ZIP 239 preparations 1

This is the first of several backports to prepare for ZIP 239. The primary
change is altering `mapRelay` to store `CTransaction`s, which we need
because ZIP 239 requires changing `Inv` messages based on transaction
versions. The other changes are mainly for conflict removal but are also
independently useful.

Backports the following upstream PRs:
- bitcoin/bitcoin#6889
- bitcoin/bitcoin#7125
- bitcoin/bitcoin#7862
- bitcoin/bitcoin#7877
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants