test: Avoid testing negative block heights#24237
Conversation
| tx.vout[0].nValue = i; //Keep txs unique unless intended to duplicate | ||
| tx.vout[0].scriptPubKey.assign(InsecureRand32() & 0x3F, 0); // Random sizes so we can test memory usage accounting | ||
| unsigned int height = InsecureRand32(); | ||
| const int height{int(InsecureRand32() >> 1)}; |
There was a problem hiding this comment.
use we just use GetRandInt from random.h?
There was a problem hiding this comment.
Might be better to do this in a separate pull request unrelated from the changes here?
There was a problem hiding this comment.
meh, what's the point of a new PR just for that? Probably no-one would review it 😂 imo, it makes much more sense here
There was a problem hiding this comment.
GetRandInt is either non-deterministic, or returns a constant (deterministic). Can you explain why either change makes sense? I'd say neither does and it should be left-as is.
There was a problem hiding this comment.
Ahh, okay, that's a good reason :) Maybe in the future we should adjust InsecureRand32 to make it clearer that it can be deterministic
fad8154 test: Avoid testing negative block heights (MarcoFalke) Pull request description: A negative chain height is only used to denote an empty chain, not the height of any block. So stop testing that and remove a suppression. ACKs for top commit: brunoerg: crACK fad8154 Tree-SHA512: 0f9e91617dfb6ceda99831e6cf4b4bf0d951054957c159b1a05a178ab6090798fae7368edefe12800da24585bcdf7299ec3534f4d3bbf5ce6a6eca74dd3bb766
Summary: This is a backport of [[bitcoin/bitcoin#24237 | core#24237]] Depends on D12819 Test Plan: With UBSAN `ninja && ninja check check-functional bitcoin-fuzzers` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Subscribers: Fabien Differential Revision: https://reviews.bitcoinabc.org/D12820
4 participants
A negative chain height is only used to denote an empty chain, not the height of any block.
So stop testing that and remove a suppression.