š¬ fjahr commented on pull request "test, assumeutxo: Add test to ensure failure when mempool not empty":
(https://github.com/bitcoin/bitcoin/pull/29394#discussion_r1483608953)
There is an extra space after the extra_args option.
Since the `-persistmempool=0` is only relevant to this one restart and not the rest of the tests I wouldn't change the setup and instead do it like this, but how it's done now is also ok.
Another small improvement might be to switch from node 2 to node 0. None of the args from node 2 are needed for this test, so using node 0, that has no args, may cause less confusion.
```suggestion
# Restart to purge the transaction from th
...
(https://github.com/bitcoin/bitcoin/pull/29394#discussion_r1483608953)
There is an extra space after the extra_args option.
Since the `-persistmempool=0` is only relevant to this one restart and not the rest of the tests I wouldn't change the setup and instead do it like this, but how it's done now is also ok.
Another small improvement might be to switch from node 2 to node 0. None of the args from node 2 are needed for this test, so using node 0, that has no args, may cause less confusion.
```suggestion
# Restart to purge the transaction from th
...
š¬ brunoerg commented on pull request "i2p: log connection was refused due to arbitrary port":
(https://github.com/bitcoin/bitcoin/pull/29393#discussion_r1483610162)
Good suggestion.
(https://github.com/bitcoin/bitcoin/pull/29393#discussion_r1483610162)
Good suggestion.
š¬ fjahr commented on pull request "test, assumeutxo: Add test to ensure failure when mempool not empty":
(https://github.com/bitcoin/bitcoin/pull/29394#discussion_r1483610267)
Actually, when you switch to use node 0 you can remove this line entirely because node 0 is not used to load snapshots in the rest of the test.
(https://github.com/bitcoin/bitcoin/pull/29394#discussion_r1483610267)
Actually, when you switch to use node 0 you can remove this line entirely because node 0 is not used to load snapshots in the rest of the test.
š¬ glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1483614824)
Hmmm I find early returns in the middle of a function a bit brittle. Would be easier to write a bug if e.g. we added another v3 rule at the bottom of the function.
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1483614824)
Hmmm I find early returns in the middle of a function a bit brittle. Would be easier to write a bug if e.g. we added another v3 rule at the bottom of the function.
š¬ fjahr commented on pull request "test, assumeutxo: Add test to ensure failure when mempool not empty":
(https://github.com/bitcoin/bitcoin/pull/29394#discussion_r1483614887)
@hernanmarino you marked the other thread as resolved while I was writing, this change works and I think that's what @maflcko had in mind: https://github.com/fjahr/bitcoin/commit/a6883b8919d7348bab56f6ebee0818912146ae52
(https://github.com/bitcoin/bitcoin/pull/29394#discussion_r1483614887)
@hernanmarino you marked the other thread as resolved while I was writing, this change works and I think that's what @maflcko had in mind: https://github.com/fjahr/bitcoin/commit/a6883b8919d7348bab56f6ebee0818912146ae52
š¬ instagibbs commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1483617248)
fair enough, I just keep forgetting I can assume it has a parent because this block is pretty large. maybe a skill issue on my part
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1483617248)
fair enough, I just keep forgetting I can assume it has a parent because this block is pretty large. maybe a skill issue on my part
š¬ dergoegge commented on pull request "RFC: build: remove confusing and inconsistent disable-asm option":
(https://github.com/bitcoin/bitcoin/pull/29407#issuecomment-1934972190)
@jonatack see https://github.com/bitcoin/bitcoin/issues/29178 (we prefer having this broken over reverting the change that introduced it for some reason)
I'm not sure if the `random` harness would trigger the ASan error, the docs mention sha256 which `random` does not use afaict. Could you try `txorphan` or a different harness that does some hashing?
Did you set `UBSAN_OPTIONS` to include `halt_on_error=1`? that is likely why it keeps running if you didn't
(https://github.com/bitcoin/bitcoin/pull/29407#issuecomment-1934972190)
@jonatack see https://github.com/bitcoin/bitcoin/issues/29178 (we prefer having this broken over reverting the change that introduced it for some reason)
I'm not sure if the `random` harness would trigger the ASan error, the docs mention sha256 which `random` does not use afaict. Could you try `txorphan` or a different harness that does some hashing?
Did you set `UBSAN_OPTIONS` to include `halt_on_error=1`? that is likely why it keeps running if you didn't
š¬ brunoerg commented on pull request "i2p: log connection was refused due to arbitrary port":
(https://github.com/bitcoin/bitcoin/pull/29393#issuecomment-1934972685)
Force-pushed addressing https://github.com/bitcoin/bitcoin/pull/29393#discussion_r1483580775
(https://github.com/bitcoin/bitcoin/pull/29393#issuecomment-1934972685)
Force-pushed addressing https://github.com/bitcoin/bitcoin/pull/29393#discussion_r1483580775
š¬ brunoerg commented on pull request "p2p: Allow whitelisting manual connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1483623515)
Sounds good, I'll address it.
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1483623515)
Sounds good, I'll address it.
š¬ brunoerg commented on pull request "p2p: Allow whitelisting manual connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1483624485)
I'll fix it
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1483624485)
I'll fix it
š¬ jonatack commented on pull request "i2p: log connection was refused due to arbitrary port":
(https://github.com/bitcoin/bitcoin/pull/29393#issuecomment-1934983441)
ACK 5b358cdd1a5f5d2fe87a9e41c638996eab2e2796
(https://github.com/bitcoin/bitcoin/pull/29393#issuecomment-1934983441)
ACK 5b358cdd1a5f5d2fe87a9e41c638996eab2e2796
š¬ glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1483628446)
done
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1483628446)
done
š¬ glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1483628863)
added
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1483628863)
added
š¬ glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1483628734)
added a comment
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1483628734)
added a comment
š¬ glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1483628675)
removed
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1483628675)
removed
š¬ glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1483628493)
done
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1483628493)
done
š¬ glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1483628403)
Gonna mark this as resolved š
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1483628403)
Gonna mark this as resolved š
š¬ glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1483631193)
I agree that in the case above D shouldn't need to conflict with all children, so will leave as is.
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1483631193)
I agree that in the case above D shouldn't need to conflict with all children, so will leave as is.
š¬ glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#issuecomment-1934997643)
> To unblock progress on other work, my suggestion is that we do the following:
>
> * modify this PR to introduce the v3 policy and validation rules, but not make v3 transactions standard for relay yet -- this should be a very small code change I think
Pushed to address @instagibbs review and make v3 not-yet-standard for mainnet.
(https://github.com/bitcoin/bitcoin/pull/28948#issuecomment-1934997643)
> To unblock progress on other work, my suggestion is that we do the following:
>
> * modify this PR to introduce the v3 policy and validation rules, but not make v3 transactions standard for relay yet -- this should be a very small code change I think
Pushed to address @instagibbs review and make v3 not-yet-standard for mainnet.
š¬ ariard commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#issuecomment-1935034851)
> To be clear, package relay with package RBF is meant to be resistant to these kinds of network topology aware (NTA) pinning scenarios. With correctly implemented package relay it doesn't matter that a CPFP tx did a CPFP on top of a commitment transaction that no-one but the originator has: provided the fee (or with RBFR, fee-rate) outbids the competing commitment transaction, it will be replaced as nodes evaluate the new package as a whole.
iām not sure if the adversary injects malicious pa
...
(https://github.com/bitcoin/bitcoin/pull/28948#issuecomment-1935034851)
> To be clear, package relay with package RBF is meant to be resistant to these kinds of network topology aware (NTA) pinning scenarios. With correctly implemented package relay it doesn't matter that a CPFP tx did a CPFP on top of a commitment transaction that no-one but the originator has: provided the fee (or with RBFR, fee-rate) outbids the competing commitment transaction, it will be replaced as nodes evaluate the new package as a whole.
iām not sure if the adversary injects malicious pa
...