Skip to content

Remove IPFS anchoring - #35

Merged
akru merged 4 commits into
mainfrom
remove-ipfs-anchoring-34
Sep 28, 2026
Merged

akru merged 4 commits into
mainfrom
remove-ipfs-anchoring-34

Conversation

@akru

@akru akru commented Sep 28, 2026

Copy link
Copy Markdown
Member

…compressed batches directly (#34)

Removes the ipfs-publisher service and the IPFS/CID indirection between batcher and blockchain-anchor. batcher now XZ-compresses each detached batch, recursively splits it to fit ANCHOR_MAX_PAYLOAD_BYTES (shared default 8192, defined in @scp/core), and emits chain-ready payloads directly on telemetry.batched.v1 (batch_id, payload, uncompressed_size, compressed_size, payload_hash). Oversized single events are routed to the DLQ instead of blocking the rest of the batch.

blockchain-anchor now consumes telemetry.batched.v1 directly and submits the compressed payload bytes on-chain via cps.setPayload. Idempotency is based on a byte-for-byte comparison of the incoming payload against the current on-chain payload (api.query.cps.payload), replacing the previous CID-string comparison. Oversized payloads are defensively rejected as a permanent error.

  • Reworked TelemetryBatchedPayload proto, removed TelemetryIpfsPublishedPayload and the ipfs.published.v1 topic
  • Deleted services/ipfs-publisher entirely
  • Updated docker-compose.yml, kafka-init-topics.sh, integration-test.yml, and .env.example to drop IPFS publisher wiring (the ipfs container is kept, since it is still used as a libp2p GossipSub reserved peer for pubsub-broadcaster integration tests)
  • Updated READMEs, architecture docs, tracing.md, and WP-04/WP-05 work package docs to reflect the new pipeline

…compressed batches directly (#34)

Removes the ipfs-publisher service and the IPFS/CID indirection between
batcher and blockchain-anchor. batcher now XZ-compresses each detached
batch, recursively splits it to fit ANCHOR_MAX_PAYLOAD_BYTES (shared
default 8192, defined in @scp/core), and emits chain-ready payloads
directly on telemetry.batched.v1 (batch_id, payload, uncompressed_size,
compressed_size, payload_hash). Oversized single events are routed to
the DLQ instead of blocking the rest of the batch.

blockchain-anchor now consumes telemetry.batched.v1 directly and
submits the compressed payload bytes on-chain via cps.setPayload.
Idempotency is based on a byte-for-byte comparison of the incoming
payload against the current on-chain payload (api.query.cps.payload),
replacing the previous CID-string comparison. Oversized payloads are
defensively rejected as a permanent error.

- Reworked TelemetryBatchedPayload proto, removed
  TelemetryIpfsPublishedPayload and the ipfs.published.v1 topic
- Deleted services/ipfs-publisher entirely
- Updated docker-compose.yml, kafka-init-topics.sh,
  integration-test.yml, and .env.example to drop IPFS
  publisher wiring (the ipfs container is kept, since it is still used
  as a libp2p GossipSub reserved peer for pubsub-broadcaster
  integration tests)
- Updated READMEs, architecture docs, tracing.md, and WP-04/WP-05 work
  package docs to reflect the new pipeline

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@akru akru self-assigned this Sep 28, 2026
@akru
akru requested a lite review from Copilot September 28, 2026 12:01

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Critical batching and anchoring failure paths, plus missing integration coverage, remain unresolved.

Review effort: Lite
Findings: 3 High severity · 1 Medium severity

Open (4)
What changed in this PR

This PR removes IPFS anchoring and sends compressed, size-limited batches directly from batcher to blockchain-anchor.

Changes:

  • Adds XZ compression, recursive splitting, hashing, and DLQ handling.
  • Updates direct byte-based blockchain anchoring and idempotency.
  • Removes the IPFS publisher, topic, wiring, and obsolete documentation.
File Summary
services/​pubsub-broadcaster/​README.md Updated archival-flow documentation.
services/​ipfs-publisher/​tsconfig.json Removed obsolete service configuration.
services/​ipfs-publisher/​tests/​providers/​pinata.test.ts Removed obsolete provider tests.
services/​ipfs-publisher/​tests/​multi-provider-client.test.ts Removed obsolete client tests.
services/​ipfs-publisher/​tests/​durable-event-identity.test.ts Removed obsolete identity tests.
services/​ipfs-publisher/​tests/​durability.test.ts Removed obsolete durability tests.
services/​ipfs-publisher/​tests/​contracts.test.ts Removed obsolete contract tests.
services/​ipfs-publisher/​tests/​config.test.ts Removed obsolete configuration tests.
services/​ipfs-publisher/​src/​providers/​types.ts Removed obsolete provider types.
services/​ipfs-publisher/​src/​providers/​pinata.ts Removed obsolete Pinata provider.
services/​ipfs-publisher/​src/​providers/​kubo.ts Removed obsolete Kubo provider.
services/​ipfs-publisher/​src/​providers/​index.ts Removed obsolete provider exports.
services/​ipfs-publisher/​src/​multi-provider-client.ts Removed obsolete provider client.
services/​ipfs-publisher/​src/​logger.ts Removed obsolete logging code.
services/​ipfs-publisher/​src/​index.ts Removed obsolete publisher entrypoint.
services/​ipfs-publisher/​src/​durability.ts Removed obsolete durability logic.
services/​ipfs-publisher/​src/​config.ts Removed obsolete publisher configuration.
services/​ipfs-publisher/​README.md Removed obsolete service documentation.
services/​ipfs-publisher/​package.json Removed obsolete service dependencies.
services/​blockchain-anchor/​tests/​shutdown.test.ts Updated anchor shutdown coverage.
services/​blockchain-anchor/​tests/​idempotency.test.ts Updated byte-based idempotency coverage.
services/​blockchain-anchor/​tests/​contracts.test.ts Updated anchoring contract tests.
services/​blockchain-anchor/​tests/​config.test.ts Updated anchor configuration tests.
services/​blockchain-anchor/​tests/​cid-encoding.test.ts Removed obsolete CID encoding coverage.
services/​blockchain-anchor/​src/​index.ts Added direct anchoring; critical DLQ and failed-offset handling findings remain.
services/​blockchain-anchor/​src/​config.ts Added payload-size configuration.
services/​blockchain-anchor/​README.md Documented direct payload anchoring.
services/​blockchain-anchor/​package.json Removed obsolete dependencies.
services/​batcher/​tests/​service.shutdown.test.ts Updated batcher shutdown coverage.
services/​batcher/​tests/​payload-builder.test.ts Added compression and payload-fitting coverage.
services/​batcher/​tests/​contracts.test.ts Updated batcher contract tests.
services/​batcher/​src/​payload-builder.ts Added compression, fitting, splitting, and oversized detection.
services/​batcher/​src/​index.ts Publishes fitted payloads; critical split-retry duplication and moderate envelope-preservation findings remain.
services/​batcher/​src/​config.ts Added shared payload-limit configuration.
services/​batcher/​src/​batch-flusher.ts Updated flush documentation.
services/​batcher/​README.md Documented chain-ready batching.
services/​batcher/​package.json Added compression and hashing dependencies.
README.md Updated service architecture documentation.
proto/​connectivity/​v1/​payload.proto Reworked the batched payload schema.
pnpm-lock.yaml Updated dependency resolution.
packages/​core/​tests/​core.test.ts Updated topic assertions.
packages/​core/​src/​topics.ts Removed the IPFS topic.
packages/​core/​src/​index.ts Exported anchor configuration.
packages/​core/​src/​anchor.ts Defined the shared payload-size default.
packages/​core/​README.md Updated schema documentation.
docs/​wp/​wp-05-blockchain-anchor.md Updated direct anchoring scope.
docs/​wp/​wp-04-ipfs-publisher.md Marked the work package superseded; stale historical sections remain.
docs/​wp/​README.md Updated work-package pipeline documentation.
docs/​tracing.md Updated batch tracing terminology.
docs/​architecture/​project-architecture.md Updated system architecture.
docker/​kafka-init-topics.sh Removed IPFS topic initialization.
docker-compose.yml Documented IPFS’s remaining test-only role.
.github/​workflows/​integration-test.yml Removed IPFS startup, but lacks batcher-to-anchor integration coverage.
.github/​copilot-instructions.md Updated architecture guidance.
.env.example Removed IPFS settings and added anchor limits.
Files not reviewed (1)
  • pnpm-lock.yaml: Generated file

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread services/batcher/src/index.ts Outdated
Comment thread services/blockchain-anchor/src/index.ts Outdated
Comment thread services/blockchain-anchor/src/index.ts Outdated
Comment thread services/batcher/src/index.ts Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

The current byte comparison breaks on SCALE encoding, Kafka publication is not atomic, and required result-event and end-to-end coverage are missing.

Review effort: Balanced
Findings: 4 High severity

Open (4)
Resolved since last review (4)
Files not reviewed (1)
  • pnpm-lock.yaml: Generated file
Previously missed (8)

In code that hasn't changed since last review

Medium severity Start batcher and verify the end-to-end anchoring flow

.github/​workflows/​integration-test.yml:218

The workflow still starts blockchain-anchor but never starts @scp/batcher, so telemetry cannot reach telemetry.batched.v1 and the new direct anchoring path is not exercised. Issue #34 explicitly requires an end-to-end byte-identity check from batcher output to set_payload; add the batcher startup/health checks and verify that anchoring flow.

Medium severity Add or provision CPS metadata for encoded compressed payloads

proto/​connectivity/​v1/​payload.proto:31

The contract says CPS metadata describes the schema, encoding, and compression, but this PR contains no metadata update/provisioning path. Without that metadata, on-chain readers cannot determine that these raw bytes are protobuf SignedEnvelopeBatch compressed with XZ, which leaves issue #34's payload-interpretation acceptance criterion unmet. Add the CPS metadata update mechanism or document and provision the required out-of-band metadata.

Medium severity Count all terminal outcomes after recursive batch splitting

services/​batcher/​src/​index.ts:243

This misses batches that were split into one fitted sub-batch plus one or more oversized events: fitted.length is then 1 even though recursive splitting occurred. Count all terminal outcomes when deciding whether the original batch split.

Medium severity Publish blockchain result before committing the offset

services/​blockchain-anchor/​src/​index.ts:515

The successful path proceeds directly to committing the consumed offset without publishing telemetry.blockchain.result.v1. Issue #34 requires a CID-free blockchain result and commit only after that result is emitted, so this currently leaves downstream consumers with no anchoring outcome. Publish the batch-based result before this commit.

Low severity Correct setup steps for the retained IPFS container

.github/​copilot-instructions.md:152

docker compose up -d still starts the retained IPFS container for the pubsub reserved peer, so this setup step is inaccurate even though IPFS is no longer part of anchoring.

Low severity Document the retained IPFS container in the setup

README.md:26

make all still runs docker compose up, and the compose file intentionally retains the IPFS container as the pubsub reserved peer. Omitting IPFS here makes the setup description inaccurate.

Low severity Include trace IDs and batch IDs in produced-batch logs

docs/​tracing.md:126

This lookup cannot return a batch_id for the trace: the batcher's fitting batch log includes trace_ids but no batch ID, while its batch produced log includes the batch ID but omits trace IDs. Add per-sub-batch trace IDs to the produced log (or revise the documented correlation procedure) so this command is actionable.

Low severity Test fitBatch at the exact compressed size limit

services/​batcher/​tests/​payload-builder.test.ts:65

This only checks a standalone array length and never exercises fitBatch with a compressed result exactly equal to the limit, so the named boundary behavior and issue #34 acceptance criterion remain untested. Use a deterministic fixture or injectable compressor that makes fitBatch return an exact-limit payload.

Comment thread services/batcher/src/index.ts Outdated
Comment thread services/blockchain-anchor/src/index.ts Outdated
Comment thread services/blockchain-anchor/src/index.ts
Comment thread services/blockchain-anchor/tests/shutdown.test.ts
akru and others added 2 commits September 29, 2026 01:02
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
- blockchain-anchor: BoundedVec<u8>.toU8a() includes the SCALE
  compact-length prefix by default; pass toU8a(true) so the on-chain
  payload bytes compared for idempotency match the bare bytes we submit.
- blockchain-anchor: compare batches by their content-addressed
  payload_hash (blake2b-256, already computed by the batcher) against a
  hash of the currently anchored on-chain payload, instead of comparing
  raw bytes. The Kafka offset for a batch is only committed once this
  check (and, if needed, the extrinsic) completes, so an uncommitted
  offset already durably marks a batch as unconfirmed — no additional
  persisted queue is required.
- batcher: update the produceBatch comment to reflect that re-publishing
  a sub-batch is harmless now that anchoring dedupes by payload_hash,
  regardless of whether the whole split lands atomically.
- tests: idempotency.test.ts now computes payload_hash the same way the
  real batcher does (blake2b-256 of the payload) instead of a fixed
  placeholder, so the hash-based dedup check is exercised correctly.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@akru
akru merged commit bc5f71b into main Sep 28, 2026
2 checks passed
@akru
akru deleted the remove-ipfs-anchoring-34 branch September 28, 2026 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants