Skip to content

Conversation

@carbolymer
Copy link
Contributor

@carbolymer carbolymer commented Sep 5, 2025

Changelog

- description: |
    Add decoded PlutusData and NativeScript in proto definition #947 
# uncomment types applicable to the change:
  type:
   - feature        # introduces a new feature
  # - breaking       # the API has changed in a breaking way
   - compatible     # the API has changed but is non-breaking
  # - optimisation   # measurable performance improvements
  # - refactoring    # QoL changes
  # - bugfix         # fixes a defect
  # - test           # fixes/modifies tests
  # - maintenance    # not directly related to the code
  # - release        # related to a new release preparation
  # - documentation  # change in code docs, haddocks...
# uncomment at least one main project this PR is associated with
  projects:
  # - cardano-api
  # - cardano-api-gen
   - cardano-rpc
  # - cardano-wasm

Context

Previously, NativeScript and PlutusData were omitted in the ReadUtxos query feature:

This PR adds decoding of those values to the protobuf type. Using protobuf-defined values instead of CBOR blobs is advantageous, as it lets UTxO RPC consumers work with a single data format.

A rountrip property test is included.

Checklist

  • Commit sequence broadly makes sense and commits have useful messages
  • New tests are added if needed and existing tests are updated. See Running tests for more details
  • Self-reviewed the diff

@carbolymer carbolymer self-assigned this Sep 5, 2025
@carbolymer carbolymer changed the title cardano-rpc | Add decoded PlutusData to Datum in proto definition cardano-rpc | Add decoded PlutusData and NativeScript in proto definition Sep 8, 2025
@carbolymer carbolymer force-pushed the mgalazyn/feature/rpc-add-plutusdata-support branch from 2a68c4a to 0da56f8 Compare September 8, 2025 14:15
@carbolymer carbolymer marked this pull request as ready for review September 8, 2025 14:45
Copy link
Contributor

@palas palas left a comment

Choose a reason for hiding this comment

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

I think it is all accurate except for the bigint representation, which I am sure you already considered. But I commented on the bits that I found suspicious, even though I think they are all correct

Copy link
Contributor

@Jimbo4350 Jimbo4350 left a comment

Choose a reason for hiding this comment

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

This LGTM however you should start implementing round trip tests for your Inject instances and consider having a separate type class for these transformations.

I'll approve once the round trip tests are added.

@carbolymer carbolymer force-pushed the mgalazyn/feature/rpc-move-address-array-to-cardano-proto branch from f857964 to a345801 Compare September 9, 2025 11:39
Base automatically changed from mgalazyn/feature/rpc-move-address-array-to-cardano-proto to master September 10, 2025 14:02
@carbolymer carbolymer force-pushed the mgalazyn/feature/rpc-add-plutusdata-support branch 2 times, most recently from 7a9b167 to 97c55d9 Compare October 2, 2025 13:32
@carbolymer carbolymer marked this pull request as draft October 2, 2025 18:28
@carbolymer carbolymer force-pushed the mgalazyn/feature/rpc-add-plutusdata-support branch from 27dd02b to 810f789 Compare October 21, 2025 19:46
@carbolymer carbolymer force-pushed the mgalazyn/feature/rpc-add-plutusdata-support branch from ca99912 to 147cb62 Compare November 6, 2025 15:22
Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

hlint found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

@carbolymer carbolymer force-pushed the mgalazyn/feature/rpc-add-plutusdata-support branch from 147cb62 to a60f3b5 Compare November 12, 2025 13:10
@carbolymer carbolymer force-pushed the mgalazyn/feature/rpc-add-plutusdata-support branch 3 times, most recently from 4caaa17 to dcf43dd Compare November 12, 2025 16:25
@carbolymer carbolymer changed the title cardano-rpc | Add decoded PlutusData and NativeScript in proto definition gRPC: cardano-rpc | Add decoded PlutusData and NativeScript in proto definition Nov 13, 2025
@carbolymer carbolymer changed the title gRPC: cardano-rpc | Add decoded PlutusData and NativeScript in proto definition cardano-rpc | Add decoded PlutusData and NativeScript in proto definition Nov 13, 2025
@carbolymer carbolymer changed the title cardano-rpc | Add decoded PlutusData and NativeScript in proto definition gRPC: Add decoded PlutusData and NativeScript in proto definition Nov 13, 2025
@carbolymer carbolymer force-pushed the mgalazyn/feature/rpc-add-plutusdata-support branch from 768f771 to ccacd4c Compare November 13, 2025 18:36
@carbolymer carbolymer force-pushed the mgalazyn/feature/rpc-add-plutusdata-support branch 9 times, most recently from ab400b4 to 1c90b69 Compare November 14, 2025 11:14
@carbolymer carbolymer force-pushed the mgalazyn/feature/rpc-add-plutusdata-support branch from 1c90b69 to 8be0f75 Compare November 14, 2025 13:53
@carbolymer carbolymer marked this pull request as ready for review November 14, 2025 14:03
@carbolymer carbolymer force-pushed the mgalazyn/feature/rpc-add-plutusdata-support branch from e1eae2b to adcef0d Compare November 14, 2025 14:04
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.

gRPC: Add decoded PlutusData and NativeScript in proto definition

4 participants