Skip to content

Conversation

@woogieboogie-jl
Copy link
Contributor

Hey team,

I've made a few minor improvements to make our local development setup smoother and more robust for transmitter users

The Problem

When I first set up the project, I ran into a few small snags:

  1. The app would fail silently because the dotenv package was missing from our dependencies.
  2. The Redis container would crash if you didn't set a password in the .env file.

The Fix

  1. I added the missing dotenv package to our package.json.
  2. I updated the docker-compose.yml so the Redis container starts up happily even if a password isn't set + minor edits like depends_on to make sure redis is running

You can test by following the steps in the original PR description. All my local checks have passed!

Thanks!

@woogieboogie-jl
Copy link
Contributor Author

woogieboogie-jl commented Jun 25, 2025

Additionally pushed dd6c46d so transmitter can pick up changes in config.yml with container restart (unless it was a design choice where changes in config.yml doesn't get picked up by live transmitter unless built anew) :)

@woogieboogie-jl
Copy link
Contributor Author

Hey team,

While integrating with DataStreamsFeed.sol we realised our transmitter
didn’t pass the two parameters expected by the on-chain updateReport method:

function updateReport(
    uint16 reportVersion,
    bytes calldata verifiedReportData
) external onlyRole(Roles.REPORT_VERIFIER);

What was missing

  • Our internal ReportV3 / ReportV4 objects had no
    reportVersion or verifiedReport fields, so
    executeContract() looked them up and received undefined,
    which made viem throw Cannot convert undefined to a BigInt.

What’s new in 9fe3951

  • types.ts – added
    reportVersion: number and verifiedReport: Hex to both ReportV3 and ReportV4.
  • services/client.ts
    • When a report is verified, we now populate those two fields.
    • No other logic changed – everything that used these objects before still works.

Why it matters

With the extra fields present the transmitter can encode and send
updateReport(reportVersion, verifiedReportData) correctly, letting us
write Data Streams prices on-chain.

Thanks!

@woogieboogie-jl
Copy link
Contributor Author

Hey team,

Two quick README tweaks in this PR:

  1. Example receiver contract added
    contracts/DataStreamsFeed.sol – Adrastia’s community implementation of a Data Streams feed reader.

  2. Safety notice
    • README now highlights that the sample contract is unaudited and ships without OZ / Chainlink deps or deployment scripts; you must install the libraries and assess risk on your own.

Tiny change, but it should save newcomers a lot of setup questions.

Thanks!

@woogieboogie-jl
Copy link
Contributor Author

Hey team, apologies on having too many lengthened PR thread - note that this pr was supposed to be limited to minor fixes / chores but - while trying to add support to our worldchain live contract (aka Adriasta's contract - happened to be a bit messier pr than expected.

on to the point - two updates with: b63c019

  • On-chain verification support

    • Transmitter now handles verifyAndUpdateReport (raw report + parameter payload).
    • config-chainlink-verify-example.yml illustrates this path (LINK-funded feed, no role needed).
    • config-chainlink-example.yml kept for off-chain verification (updateReport) and documents REPORT_VERIFIER role requirement.
  • Config comments & docs

    • Added banner comments to both sample configs to spell out when to use each mode and the operational caveats (LINK balance vs. role assignment).

Should make setup choices much clearer with minimal code churn. Thanks!

@mradkov
Copy link
Member

mradkov commented Jul 1, 2025

Hi @woogieboogie-jl

Thank you for your contribution. We've fixed some of the issues identified in your PR in the previous PR #17 which is currently undergoing security review.

We will merge it soon and ask you to rebase you PR and only pick the things which are not already fixed by the previous pr.

@AtanasKrondev
Copy link
Member

Can you rebase as we've merged some changes into master.

In general the PR looks good, but we need to check if we can include the contracts related to your oracle service, for business reasons.

@woogieboogie-jl
Copy link
Contributor Author

yup will do soon - will have the contract features in separate commits.

@woogieboogie-jl
Copy link
Contributor Author

Closing this gig as it is superseeded by PR#24

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.

3 participants