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.

Sign up for GitHub

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

Jump to bottom

2023 05 tx compression #3

Closed
wants to merge 1,825 commits into from
Closed

2023 05 tx compression #3

wants to merge 1,825 commits into from

Conversation

TomBriar
Copy link
Owner

@TomBriar TomBriar commented Jul 4, 2023

No description provided.

TomBriar added a commit that referenced this pull request Jul 4, 2023
@TomBriar
Cleaning up PR and removing unneeded changes, #3
f3dd245
TomBriar added a commit that referenced this pull request Jul 4, 2023
@TomBriar
Cleaning up PR and removing unneeded changes, #3
593b907
apoelstra
apoelstra reviewed Jul 11, 2023
View reviewed changes
src/core_read.cpp Outdated Show resolved Hide resolved
apoelstra
apoelstra reviewed Jul 11, 2023
View reviewed changes
src/primitives/block.h Outdated Show resolved Hide resolved
TomBriar added a commit that referenced this pull request Jul 11, 2023
@TomBriar
Cleaning up PR and removing unneeded changes, #3
9cb3d1f
@TomBriar TomBriar force-pushed the 2023-05--tx-compression branch 3 times, most recently from 56e16b9 to 511cda6 Compare July 11, 2023 21:12
@apoelstra
Copy link

37ef521 still uses tabs for spacing.

@apoelstra
Copy link

So does 3985145 -- 3985145 also has trailing spaces.

@TomBriar TomBriar force-pushed the 2023-05--tx-compression branch 3 times, most recently from 1cd53ba to a434a5d Compare July 24, 2023 18:39
@TomBriar TomBriar force-pushed the 2023-05--tx-compression branch 2 times, most recently from b7724d2 to 62532dc Compare August 1, 2023 18:10
ismaelsadeeq and others added 6 commits December 19, 2023 17:12
@ismaelsadeeq
[doc]: add doxygen return comment for CheckPackageLimits
19bb65b
@achow101
Merge bitcoin#29037: Add multiplication operator to CFeeRate
e3847f7 
1757452 test: Add tests for CFeeRate multiplication operator (Kashif Smith)
1553c80 Add multiplication operator to CFeeRate (Murch)

Pull request description:

  Allows us to use
  `coin_selection_params.m_long_term_feerate * 3`
  or
  `3 * coin_selection_params.m_long_term_feerate`
  instead of
  `CFeeRate{coin_selection_params.m_long_term_feerate.GetFee(3000)}`

  inspired by bitcoin#27877 (comment)

ACKs for top commit:
  kevkevinpal:
    reACK [1757452](bitcoin@1757452)
  achow101:
    ACK 1757452
  ajtowns:
    ACK 1757452 ; lgtm
  ismaelsadeeq:
    ACK 1757452

Tree-SHA512: a86faac1efd1b7688630cd811246533d184d56b62064a7fd9007de95dbf81fa668aa2252253d102fba67517b6a4ca2dc367c5388b8ab936215734d7d370740cf
@glozow
Merge bitcoin#29115: [doc]: add doxygen comment describing what `Chec…
3a0f54d 
…kPackageLimits` returns

19bb65b [doc]: add doxygen return comment for CheckPackageLimits (ismaelsadeeq)

Pull request description:

  This PR adds a  doxygen comment on `CheckPackageLimits` describing what the method returns.

  Fixes bitcoin#28863 (comment)

ACKs for top commit:
  Sjors:
    utACK 19bb65b
  Zero-1729:
    utACK 19bb65b

Tree-SHA512: ccf1cc00a44d3fff60f28ad6766019a9f61b349729eab3cb02bc76b13c2e55441348a1602d806e60e4b2eabeb1f5d1ddacddf86c0bcdb78b078bb3a863b650c2
@achow101
Merge bitcoin#28372: fuzz: coinselection, improve min_viable_change…
7524fcf 
…/`change_output_size`

cd81007 fuzz: coinselection, improve `min_viable_change`/`change_output_size` (brunoerg)

Pull request description:

  Instead of "randomly" fuzzing `min_viable_change` and `change_output_size`, and since they're correlated, this PR changes the approach to fuzz them according to the logic in `CreateTransactionInternal`.

ACKs for top commit:
  murchandamus:
    ACK cd81007
  achow101:
    ACK cd81007
  furszy:
    Code ACK cd81007

Tree-SHA512: 4539b469f00cdf666078d80c07ed062726f804e390400348148cd3092db9cdc178c6d00ead39aef19acf97badfb6576ce23546d8967387e81c5398d52d7f4404
@achow101
Merge bitcoin#29027: wallet: fix key parsing check for miniscript exp…
eefe4ba 
…ressions

e1281f1 wallet: fix key parsing check for miniscript expressions in `ParseScript` (brunoerg)

Pull request description:

  In `ParseScript`, when processing miniscript expressions, the way we check for key parsing error is wrong, the actual code is unreachable because we're checking it into `if (node)` (successful parsing) statement.

ACKs for top commit:
  sipa:
    utACK e1281f1
  RandyMcMillan:
    utACK bitcoin@e1281f1
  achow101:
    ACK e1281f1

Tree-SHA512: c4b3765d32673928a1f6d84ecbaa311870da9a9625753ed15ea57c802a9f16ddafa48c1dc66c0e4be284c5862e7821ed94135498ed9b9f3d7342a080035da289
@achow101
Merge bitcoin#29056: refactor: Print verbose serialize compiler error…
dca0f23 
… messages

fae5263 Allow std::byte C-style array serialization (MarcoFalke)
fa898e6 refactor: Print verbose serialize compiler error messages (MarcoFalke)

Pull request description:

  Currently, trying to serialize an object that can't be serialized will fail with a short error message. For example, the diff and the error message:

  ```diff
  diff --git a/src/test/serialize_tests.cpp b/src/test/serialize_tests.cpp
  index d75eb499b4..773f49845b 100644
  --- a/src/test/serialize_tests.cpp
  +++ b/src/test/serialize_tests.cpp
  @@ -62,6 +62,8 @@ public:

   BOOST_AUTO_TEST_CASE(sizes)
   {
  +    int b[4];
  +    DataStream{} << b << Span{b};
       BOOST_CHECK_EQUAL(sizeof(unsigned char), GetSerializeSize((unsigned char)0));
       BOOST_CHECK_EQUAL(sizeof(int8_t), GetSerializeSize(int8_t(0)));
       BOOST_CHECK_EQUAL(sizeof(uint8_t), GetSerializeSize(uint8_t(0)));
  ```

  ```
  ./serialize.h:765:6: error: member reference base type 'const int[4]' is not a structure or union
    765 |     a.Serialize(os);
        |     ~^~~~~~~~~~
  ```
  ```
  ./serialize.h:277:109: error: no matching function for call to 'UCharCast'
    277 | template <typename Stream, typename B> void Serialize(Stream& s, Span<B> span) { (void)/* force byte-type */UCharCast(span.data()); s.write(AsBytes(span)); }
        |                                                                                                             ^~~~~~~~~
  ```

  This is fine. However, it would be more helpful for developers and more accurate by the compiler to explain why each function is not selected.

  Fix this by using C++20 concepts where appropriate.

ACKs for top commit:
  ajtowns:
    reACK fae5263
  achow101:
    ACK fae5263
  TheCharlatan:
    Re-ACK fae5263

Tree-SHA512: e03a684ccfcc5fbcad7f8a4899945a05989b555175fdcaebdb113aff46b52b4ee7b467192748edf99c5c348a620f8e52ab98bed3f3fca88280a64dbca458fe8a
@TomBriar TomBriar force-pushed the 2023-05--tx-compression branch 3 times, most recently from d03ebbb to ed0e798 Compare December 22, 2023 17:38
@TomBriar
Copy link
Owner Author

@apoelstra Should the ScriptWitness be limited to 10000 bytes from the MAX_SCRIPT_SIZE variable found in src/scripts/scripts.h?

@TomBriar TomBriar force-pushed the 2023-05--tx-compression branch 6 times, most recently from 79b1bf8 to c1cceb3 Compare December 26, 2023 18:50
@TomBriar TomBriar force-pushed the 2023-05--tx-compression branch 2 times, most recently from 0430476 to 1ecfbd7 Compare January 2, 2024 14:04
@TomBriar TomBriar closed this Jan 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@apoelstra apoelstra apoelstra left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone