PR 1 Add Vaults V2 protocol buffers and generated code by zmanian · Pull Request #60 · noble-assets/dollar

zmanian · 2025-10-14T21:37:22Z

This PR establishes the API contract for the Vaults V2 system by adding:

Protocol buffer definitions for all Vaults V2 messages and queries
Generated Go code (pulsar and standard protobuf types)
Type definitions for cross-chain operations, NAV tracking, and oracle management
Updated go.mod dependencies for protobuf support

Files included:

proto/noble/dollar/vaults/v2/*.proto (8 proto files)
api/vaults/v2/*.go (13 generated files)
types/vaults/v2/*.go (13 generated files + codec/keys/errors)
go.mod updates
.gitignore updates

This is the foundation for all subsequent Vaults V2 implementation work.

Consists of minor changes to protobuf files. The NAV update payload and generated code.

This PR establishes the API contract for the Vaults V2 system by adding: - Protocol buffer definitions for all Vaults V2 messages and queries - Generated Go code (pulsar and standard protobuf types) - Type definitions for cross-chain operations, NAV tracking, and oracle management - Updated go.mod dependencies for protobuf support Files included: - proto/noble/dollar/vaults/v2/*.proto (8 proto files) - api/vaults/v2/*.go (13 generated files) - types/vaults/v2/*.go (13 generated files + codec/keys/errors) - go.mod updates - .gitignore updates This is the foundation for all subsequent Vaults V2 implementation work.

coderabbitai · 2025-10-14T21:37:35Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

✨ Finishing touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

jtieri · 2025-10-15T19:27:12Z

 coverage.html
 go.work
 go.work.sum
+.gocache


why do we need to add this to .gitignore? to the best of my knowledge we don't configure the project to use a build cache in the root of the directory so the location of the build cache should be system dependent and vary depending on what you have configured in the GOCACHE env var

My guess is GOCACHE was set in-line at some point when Zaki was working. Should we add globbing that will ignore it in any path?

jtieri · 2025-10-15T19:27:48Z

 	sigs.k8s.io/yaml v1.4.0 // indirect
 )
+
+replace github.com/gogo/protobuf/grpc => ./third_party/github.com/gogo/protobuf/grpc


why do we need this replace?

We do not. Removing.

jtieri · 2025-10-15T19:31:56Z

+  uint32 route_id = 2;
+
+  // Operation type (DEPOSIT_TO_POSITION, WITHDRAWAL_FROM_POSITION, etc.)
+  string operation_type = 3;


didn't we decide to use an enum for the operation type in the previous PR dealing with protos?

jtieri · 2025-10-15T19:36:52Z

+  uint32 route_id = 2;
+
+  // Previous status
+  string previous_status = 3;


i recall using enums for some sort of status in the previous PR as well, not sure if this is the same thing or not.

jtieri · 2025-10-15T19:37:05Z

+  string previous_status = 3;
+
+  // New status
+  string new_status = 4;


jtieri · 2025-10-15T19:45:12Z

+  ];
+
+  // Hours overdue
+  int64 hours_overdue = 4;


i think previously hours_overdue was used in another Protobuf message but with type double. perhaps that is changed elsewhere in this PR but otherwise we should be consistent

jtieri · 2025-10-15T19:48:09Z

+  string current_status = 6;
+
+  // Recommended action
+  string recommended_action = 7;


Will this be some value taken from a predetermined set of actions or will it be some arbitrary string data that provides context to maintainers/operators? If it is the former then imo we should consider using an enum here to clearly define the available actions.

jtieri · 2025-10-15T20:14:30Z

+}
+
+func RegisterInterfaces(registry codectypes.InterfaceRegistry) {
+	msgservice.RegisterMsgServiceDesc(registry, &_Msg_serviceDesc)


shouldn't we register the implementations for each of our custom message types that implement sdk.Msg?

jtieri · 2025-10-15T20:17:27Z

@@ -0,0 +1,6 @@
+module github.com/gogo/protobuf/grpc


Why do we need this new go.mod file? I'm actually a bit confused as to why we need this third_party directory at all.

protobuf/grpc isn't needed, I'm removing it.

the third_party/ directory is due to the hyperlane protos not being present in buf.build.

jtieri · 2025-10-15T20:28:28Z

+	var ackID uint64
+	if len(body) == navPayloadWithAck {
+		ackID = binary.BigEndian.Uint64(body[105:113])
+	}


i'm not sure if it's documented somewhere but it could be worthwhile to make note somewhere that essentially 0 is an invalid value for an acknowledgement ID or is used to express the fact that an acknowledgement ID does not exist.

…alled)

g-luca · 2025-10-17T07:44:37Z

+  // Whether user wants to receive yield for this deposit.
+  bool receive_yield = 3;
+
+  // Set to true when the deposit should override the current preference.
+  bool receive_yield_override = 4;


i'm not entirely sure to understand why we need to use 2 mandatory booleans instead of a single one but optional . Is there a specific technical reason?

g-luca · 2025-10-17T07:58:55Z

maybe it is already planned to do it a future PR, but we should definitely have extensive unit tests around the parsing of the payload!

g-luca · 2025-10-17T08:00:58Z

+	navPayloadSize       = 105
+	navPayloadWithAck    = 113
+	navUpdateMessageType = 0x01
+	navDecimalPrecision  = int64(1_000_000_000_000_000_000) // 1e18 precision


nit

Suggested change

navDecimalPrecision = int64(1_000_000_000_000_000_000) // 1e18 precision

navDecimalPrecision = int64(1e18) // 1e18 precision

g-luca · 2025-10-17T08:11:31Z

+
+	sharePriceBig := new(big.Int).SetBytes(body[33:65])
+	sharesHeldBig := new(big.Int).SetBytes(body[65:97])
+	timestamp := int64(binary.BigEndian.Uint64(body[97:105]))


We are casting a possible value > uint64 to an int64, which can lead to negative and non-sense values, especially as we don't validate the timestamp value here

g-luca · 2025-10-17T08:33:36Z

+	positionBig := new(big.Int).SetBytes(body[1:33])
+	if !positionBig.IsUint64() {
+		return NAVUpdatePayload{}, fmt.Errorf("position identifier exceeds uint64 range")
+	}
+
+	sharePriceBig := new(big.Int).SetBytes(body[33:65])
+	sharesHeldBig := new(big.Int).SetBytes(body[65:97])


not sure if we are planning to add validation in another place or in the contract, but we are not checking if the bytes are signed, and SetBytes(...) always casts to unsigned .

g-luca · 2025-10-17T08:49:53Z

+
+const SubmoduleName = "dollar/vaults/v2"
+
+var (


nit: could we sort/organize them? So, for example, to have all the remote_position indexes/keys together? 😄

zmanian · 2025-10-23T03:09:33Z

Closing in favor of simplified 3-PR approach with updated implementation

zmanian mentioned this pull request Oct 14, 2025

Pr02: state management #61

Closed

jtieri reviewed Oct 15, 2025

View reviewed changes

cbrit added 5 commits October 16, 2025 09:59

Use OperationType enum in previously missed event fields

6c99441

Remove unnecessary gogo/protobuf/grpc vendoring

94d88bc

Change hours_overdue type int64 -> double

c731e78

Use existing enum in event status fields

d0f286f

Include V2 vault messages in RegisterInterfaces function (yet to be c…

5a44020

…alled)

g-luca reviewed Oct 17, 2025

View reviewed changes

zmanian closed this Oct 23, 2025

zmanian reopened this Oct 23, 2025

g-luca closed this Apr 7, 2026

	navDecimalPrecision = int64(1_000_000_000_000_000_000) // 1e18 precision
	navDecimalPrecision = int64(1e18) // 1e18 precision

Conversation

zmanian commented Oct 14, 2025

Uh oh!

coderabbitai Bot commented Oct 14, 2025

Review skipped

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zmanian commented Oct 23, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants