Fix DEB and RPM package compression for smaller installer sizes#20689
Open
Fix DEB and RPM package compression for smaller installer sizes#20689
Conversation
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Broken transactions never expire with few peers
- Fixed by always incrementing the sent counter on every call to increment_sent, ensuring transactions reach the expiration threshold even with fewer than 6 unique peers.
- ✅ Fixed: Duplicate peer-status check discards changed error details
- Fixed by updating the error field on duplicate (peer, status) pairs instead of returning early, ensuring fee errors on retries are captured for is_valid() checks.
Or push these changes by commenting:
@cursor push aab7cc719a
Preview (aab7cc719a)
diff --git a/build_scripts/build_linux_deb-2-installer.sh b/build_scripts/build_linux_deb-2-installer.sh
--- a/build_scripts/build_linux_deb-2-installer.sh
+++ b/build_scripts/build_linux_deb-2-installer.sh
@@ -69,7 +69,7 @@
cp -r dist/daemon/* "dist/$CLI_DEB_BASE/opt/chia/"
ln -s ../../opt/chia/chia "dist/$CLI_DEB_BASE/usr/bin/chia"
-dpkg-deb --build --root-owner-group "dist/$CLI_DEB_BASE"
+dpkg-deb -Zxz -z6 --build --root-owner-group "dist/$CLI_DEB_BASE"
# CLI only .deb done
cp -r dist/daemon ../chia-blockchain-gui/packages/gui
diff --git a/build_scripts/build_linux_rpm-2-installer.sh b/build_scripts/build_linux_rpm-2-installer.sh
--- a/build_scripts/build_linux_rpm-2-installer.sh
+++ b/build_scripts/build_linux_rpm-2-installer.sh
@@ -87,7 +87,7 @@
--rpm-tag '%undefine _missing_build_ids_terminate_build' \
--before-install=assets/rpm/before-install.sh \
--rpm-tag 'Requires(pre): findutils' \
- --rpm-compression xzmt \
+ --rpm-compression xz \
--rpm-compression-level 6 \
.
# CLI only rpm done
diff --git a/build_scripts/electron-builder.json b/build_scripts/electron-builder.json
--- a/build_scripts/electron-builder.json
+++ b/build_scripts/electron-builder.json
@@ -78,6 +78,7 @@
"deb": {
"afterInstall": "../../../build_scripts/assets/deb/postinst.sh",
"afterRemove": "../../../build_scripts/assets/deb/prerm.sh",
+ "fpm": ["--deb-compression", "xz", "--deb-compression-level", "6"],
"depends": [
"libgbm1",
"libgtk-3-0",
@@ -103,7 +104,9 @@
"--rpm-tag=Recommends: libxcrypt-compat",
"--directories=/opt/chia",
"--rpm-tag=Requires(pre): findutils",
- "--before-install=../../../build_scripts/assets/rpm/before-install.sh"
+ "--before-install=../../../build_scripts/assets/rpm/before-install.sh",
+ "--rpm-compression=xz",
+ "--rpm-compression-level=6"
]
}
}
diff --git a/chia/_tests/wallet/test_transaction_store.py b/chia/_tests/wallet/test_transaction_store.py
--- a/chia/_tests/wallet/test_transaction_store.py
+++ b/chia/_tests/wallet/test_transaction_store.py
@@ -121,12 +121,12 @@
assert await store.increment_sent(tr1.name, "peer1", MempoolInclusionStatus.SUCCESS, None) is True
tr = await store.get_transaction_record(tr1.name)
- assert tr.sent == 1
+ assert tr.sent == 2 # sent increments on every call to track total attempts
assert tr.sent_to == [("peer1", uint8(2), None), ("peer1", uint8(1), None)]
assert await store.increment_sent(tr1.name, "peer2", MempoolInclusionStatus.SUCCESS, None) is True
tr = await store.get_transaction_record(tr1.name)
- assert tr.sent == 2
+ assert tr.sent == 3 # sent increments on every call to track total attempts
assert tr.sent_to == [("peer1", uint8(2), None), ("peer1", uint8(1), None), ("peer2", uint8(1), None)]
@@ -146,6 +146,51 @@
assert tr.sent_to == [("peer1", uint8(3), "MEMPOOL_NOT_INITIALIZED")]
+@pytest.mark.anyio
+async def test_increment_sent_duplicate_peer_status_updates_error() -> None:
+ """Duplicate (peer, status) updates error if different but doesn't add to sent_to. Sent always increments."""
+ async with DBConnection(1) as db_wrapper:
+ store = await WalletTransactionStore.create(db_wrapper, MINIMUM_CONFIG)
+
+ await store.add_transaction_record(tr1)
+
+ assert await store.increment_sent(tr1.name, "peer1", MempoolInclusionStatus.PENDING, None) is True
+ tr = await store.get_transaction_record(tr1.name)
+ assert tr.sent == 1
+ assert tr.sent_to == [("peer1", uint8(2), None)]
+
+ # Same (peer, status) again: sent increments (to track attempts), no new entry in sent_to
+ assert await store.increment_sent(tr1.name, "peer1", MempoolInclusionStatus.PENDING, None) is True
+ tr = await store.get_transaction_record(tr1.name)
+ assert tr.sent == 2 # sent increments to track total attempts for expiration
+ assert tr.sent_to == [("peer1", uint8(2), None)]
+
+ # Same peer, different status: new entry in sent_to, sent increments
+ assert await store.increment_sent(tr1.name, "peer1", MempoolInclusionStatus.SUCCESS, None) is True
+ tr = await store.get_transaction_record(tr1.name)
+ assert tr.sent == 3
+ assert tr.sent_to == [("peer1", uint8(2), None), ("peer1", uint8(1), None)]
+
+ # Duplicate (peer1, SUCCESS): sent increments, no new entry
+ assert await store.increment_sent(tr1.name, "peer1", MempoolInclusionStatus.SUCCESS, None) is True
+ tr = await store.get_transaction_record(tr1.name)
+ assert tr.sent == 4
+ assert tr.sent_to == [("peer1", uint8(2), None), ("peer1", uint8(1), None)]
+
+ # Test error update: same (peer, status) with different error updates the error in sent_to
+ assert await store.increment_sent(tr1.name, "peer1", MempoolInclusionStatus.FAILED, Err.MEMPOOL_NOT_INITIALIZED) is True
+ tr = await store.get_transaction_record(tr1.name)
+ assert tr.sent == 5
+ assert tr.sent_to == [("peer1", uint8(2), None), ("peer1", uint8(1), None), ("peer1", uint8(3), "MEMPOOL_NOT_INITIALIZED")]
+
+ # Same (peer, FAILED) with fee error: error should be updated to capture fee errors on retries
+ assert await store.increment_sent(tr1.name, "peer1", MempoolInclusionStatus.FAILED, Err.INVALID_FEE_LOW_FEE) is True
+ tr = await store.get_transaction_record(tr1.name)
+ assert tr.sent == 6
+ # Error updated from MEMPOOL_NOT_INITIALIZED to INVALID_FEE_LOW_FEE
+ assert tr.sent_to == [("peer1", uint8(2), None), ("peer1", uint8(1), None), ("peer1", uint8(3), "INVALID_FEE_LOW_FEE")]
+
+
def test_filter_ok_mempool_status() -> None:
assert filter_ok_mempool_status([("peer1", uint8(1), None)]) == []
assert filter_ok_mempool_status([("peer1", uint8(2), None)]) == []
@@ -812,13 +857,15 @@
@pytest.mark.anyio
async def test_transaction_record_is_valid() -> None:
invalid_attempts: list[tuple[str, uint8, str | None]] = []
+ sent_count = 0
# The tx should be valid as long as we don't have minimum_send_attempts failed attempts
while len(invalid_attempts) < minimum_send_attempts:
- assert dataclasses.replace(tr1, sent_to=invalid_attempts).is_valid()
+ assert dataclasses.replace(tr1, sent=uint32(sent_count), sent_to=invalid_attempts).is_valid()
invalid_attempts.append(("peer", uint8(MempoolInclusionStatus.FAILED), None))
+ sent_count += 1
# The tx should be invalid now with more than minimum failed attempts
assert len(invalid_attempts) == minimum_send_attempts
- assert not dataclasses.replace(tr1, sent_to=invalid_attempts).is_valid()
+ assert not dataclasses.replace(tr1, sent=uint32(sent_count), sent_to=invalid_attempts).is_valid()
mempool_success = ("success", uint8(MempoolInclusionStatus.SUCCESS), None)
low_fee = ("low_fee", uint8(MempoolInclusionStatus.FAILED), Err.INVALID_FEE_LOW_FEE.name)
close_to_zero = (
@@ -827,9 +874,9 @@
Err.INVALID_FEE_TOO_CLOSE_TO_ZERO.name,
)
# But it should become valid with one of the above attempts
- assert dataclasses.replace(tr1, sent_to=[*invalid_attempts, mempool_success]).is_valid()
- assert dataclasses.replace(tr1, sent_to=[*invalid_attempts, low_fee]).is_valid()
- assert dataclasses.replace(tr1, sent_to=[*invalid_attempts, close_to_zero]).is_valid()
+ assert dataclasses.replace(tr1, sent=uint32(sent_count), sent_to=[*invalid_attempts, mempool_success]).is_valid()
+ assert dataclasses.replace(tr1, sent=uint32(sent_count), sent_to=[*invalid_attempts, low_fee]).is_valid()
+ assert dataclasses.replace(tr1, sent=uint32(sent_count), sent_to=[*invalid_attempts, close_to_zero]).is_valid()
@pytest.mark.anyio
diff --git a/chia/wallet/transaction_record.py b/chia/wallet/transaction_record.py
--- a/chia/wallet/transaction_record.py
+++ b/chia/wallet/transaction_record.py
@@ -80,15 +80,16 @@
return None
def is_valid(self) -> bool:
- if len(self.sent_to) < minimum_send_attempts:
- # we haven't tried enough peers yet
- return True
if any(x[1] == MempoolInclusionStatus.SUCCESS.value for x in self.sent_to):
# we managed to push it to mempool at least once
return True
if any(x[2] in {Err.INVALID_FEE_LOW_FEE.name, Err.INVALID_FEE_TOO_CLOSE_TO_ZERO.name} for x in self.sent_to):
# we tried to push it to mempool and got a fee error so it's a temporary error
return True
+ if self.sent < minimum_send_attempts:
+ # we haven't tried enough peers yet
+ return True
+
return False
def hint_dict(self) -> dict[bytes32, bytes32]:
diff --git a/chia/wallet/wallet_transaction_store.py b/chia/wallet/wallet_transaction_store.py
--- a/chia/wallet/wallet_transaction_store.py
+++ b/chia/wallet/wallet_transaction_store.py
@@ -179,6 +179,9 @@
) -> bool:
"""
Updates transaction sent count (Full Node has received spend_bundle and sent ack).
+ Each peer may contribute at most one entry per MempoolInclusionStatus; duplicate
+ (peer, status) pairs only update the error field (to capture fee errors on retries).
+ The sent counter is always incremented to track total attempts for expiration.
"""
current: TransactionRecord | None = await self.get_transaction_record(tx_id)
@@ -187,20 +190,26 @@
sent_to = current.sent_to.copy()
- current_peers = set()
err_str = err.name if err is not None else None
append_data = (name, uint8(send_status.value), err_str)
- for peer_id, status, error in sent_to:
- current_peers.add(peer_id)
+ # Check if this peer already reported this status
+ duplicate_idx: int | None = None
+ for i, (peer_id, status, _) in enumerate(sent_to):
+ if peer_id == name and status == send_status.value:
+ duplicate_idx = i
+ break
- if name in current_peers:
- sent_count = uint32(current.sent)
+ # Always increment sent to track total attempts for expiration threshold
+ sent_count = uint32(current.sent + 1)
+
+ if duplicate_idx is not None:
+ # Update error if different to capture fee errors on retries
+ if sent_to[duplicate_idx][2] != err_str:
+ sent_to[duplicate_idx] = append_data
else:
- sent_count = uint32(current.sent + 1)
+ sent_to.append(append_data)
- sent_to.append(append_data)
-
tx: TransactionRecord = dataclasses.replace(current, sent=sent_count, sent_to=sent_to)
if not tx.is_valid():
# if the tx is not valid due to repeated failures, we will confirm that we can't spend itThis Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.
DEB packages were using poor compression defaults: the GUI DEB used electron-builder's 7-Zip xz with a 4 MiB dictionary, and the CLI DEB used the build host's default zstd level 3. Both RPMs lacked explicit single-threaded xz settings. All four packages now use xz level 6 (64 MiB dictionary) for consistent, well-compressed output. Made-with: Cursor
791b177 to
0c6ac2d
Compare
Level 6 only gives an 8 MiB dictionary. Level 9 gives 64 MiB, matching the compression ratio seen in the existing GUI RPM. Made-with: Cursor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
046880e to
5e4a58d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


Summary
-Zxz -z9todpkg-deb(was using build host default zstd level 3)--deb-compression xz --deb-compression-level 9to electron-builder fpm options (was using electron-builder's 7-Zip xz with a 4 MiB dictionary)--rpm-compression xzmtto--rpm-compression xzand level from 6 to 9 (single-threaded for better cross-file deduplication, 64 MiB dictionary)--rpm-compression=xz --rpm-compression-level=9to electron-builder fpm options (was relying on fpm/rpmbuild defaults)All four Linux packages now explicitly use single-threaded xz at level 9 (64 MiB LZMA2 dictionary).
Results
Root cause
dpkg-debon the Ubuntu 22.04 build host defaults to zstd level 3, not xzxzmt(multi-threaded xz) splits the payload into small 24 MB blocks, preventing effective deduplication of the 15 near-identical ~16 MB PyInstaller executablesTest plan
data.tar.xzuses xz with 64 MiB dictionary