Skip to content

codec: create codec module#435

Open
jmank88 wants to merge 9 commits into
developfrom
codec-module
Open

codec: create codec module#435
jmank88 wants to merge 9 commits into
developfrom
codec-module

Conversation

@jmank88

@jmank88 jmank88 commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

This PR creates a separate, top-level module codec, which contains the relayer/codec and relayer/chainreader/loop packages. The old packages are deprecated but remain compatible by forwarding to the new module.

Supports:

TODO:

  • Does CI run the unit tests in their new location?
  • Split into two PRs to satisfy CI check for non-default branch commit hashes

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

These tests use packages from the main module which pollutes the go.mod. We can either leave the tests behind the main module only, or find a way to adapt them.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

we can keep them in the main module since they're technically integration-ish tests (they test CR & Indexer functionality indirectly)

Comment thread codec/types.go

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

types are migrated here

@faisal-chainlink faisal-chainlink Jul 3, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I wonder if those types belong to the top-level codec module, not a hard opinion though, I can see it working both here and in relayer/common

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.

Pull request overview

This PR factors Sui codec-related functionality into a new top-level Go module github.com/smartcontractkit/chainlink-sui/codec, moving the existing relayer/codec and relayer/chainreader/loop implementations there while keeping the old packages as deprecated forwarders for compatibility.

Changes:

  • Introduces a new codec module containing JSON decoding/type conversion, BCS conversion, encoding, and LOOP chain reader logic (plus unit/integration tests).
  • Converts existing relayer/codec and relayer/chainreader/loop packages into deprecated wrappers that forward to the new module.
  • Updates root and submodule go.mod files (and go.md dependency diagram) to depend on/replace the new codec module.

Reviewed changes

Copilot reviewed 25 out of 28 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
scripts/go.mod Adds local replace + indirect require for the new codec module.
integration-tests/go.mod Adds local replace + indirect require for the new codec module.
deployment/go.mod Adds local replace + indirect require for the new codec module.
go.mod Adds codec module dependency and local replace to ./codec.
go.md Updates module dependency diagrams to include chainlink-sui/codec.
codec/go.mod New standalone codec module definition and dependencies.
codec/go.sum New codec module sums.
codec/types.go New shared codec types + Sui address helpers + event structs.
codec/json_decoder.go New mapstructure-based JSON return decoder.
codec/type_converters.go New type-conversion registry + mapstructure decode hook.
codec/type_converters_test.go Unit tests for type conversion behaviors.
codec/bcs_converter.go New BCS primitive/vector conversion registry.
codec/bcs_converter_test.go Unit tests for BCS converter behaviors.
codec/encoder.go New encoding logic for Move/Sui argument values.
codec/decoder.go New decoding utilities + execution report deserialization.
codec/decoder_test.go Adjusts base64 decode test to not depend on removed shared.DecodeBase64.
codec/loop/loop_reader.go New LOOP chain reader implementation under codec/loop.
codec/loop/loop_reader_test.go Adds tests for LOOP reader boundary behaviors (currently build-tagged integration).
relayer/codec/types.go Converts relayer codec types into aliases/forwarders to codec.
relayer/codec/type_converters.go Converts relayer type converter into deprecated aliases/forwarders to codec.
relayer/codec/encoder.go Converts relayer encoder into deprecated forwarder to codec.
relayer/codec/decoder.go Converts relayer decoder into deprecated forwarders to codec.
relayer/codec/bcs_converter.go Converts relayer BCS converter into deprecated forwarders to codec.
relayer/chainreader/loop/loop_reader.go Converts relayer LOOP reader into deprecated forwarder to codec/loop.
relayer/chainreader/loop/loop_reader_test.go Updates test to use codec/loop implementation directly.
bindings/bind/utils.go Deprecates/forwards Sui address helpers to codec.
bindings/bind/json_decoder.go Deprecates/forwards JSON return decoding to codec.
bindings/bind/common.go Aliases bind.Object to codec.Object for backward compatibility.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread codec/loop/loop_reader_test.go
Comment thread codec/loop/loop_reader_test.go Outdated
Comment thread codec/types.go
Comment thread codec/loop/loop_reader.go
Comment thread bindings/bind/utils.go
@FelixFan1992 FelixFan1992 marked this pull request as ready for review July 3, 2026 14:45
@FelixFan1992 FelixFan1992 requested a review from a team as a code owner July 3, 2026 14:45
@faisal-chainlink

Copy link
Copy Markdown
Collaborator

This is much cleaner, thanks for creating the PR!

Small heads up that I might make some minor changes to the codec in the next couple of days as I complete the o11y integration.

@FelixFan1992

Copy link
Copy Markdown
Collaborator

This is much cleaner, thanks for creating the PR!

Small heads up that I might make some minor changes to the codec in the next couple of days as I complete the o11y integration.

can we merge this so your change is on top of this?

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.

4 participants