Skip to content

[pr] Serialize structs for svm-rollup module#4

Merged
DudessaPr merged 3 commits intofeat/risc0from
feat/serialize
Jul 30, 2024
Merged

[pr] Serialize structs for svm-rollup module#4
DudessaPr merged 3 commits intofeat/risc0from
feat/serialize

Conversation

@DudessaPr
Copy link
Copy Markdown

Problem

Some structs have to be Serializable to be included in SVM module in svm-rollup repository

Summary of Changes

Serialized ComputeBudget, FeatureSet, FeeStructure and FeeBin (which is within FeeStructure)

Fixes #

@DudessaPr DudessaPr changed the title Feat/serialize [pr] Serialize structs for svm-rollup module Jul 29, 2024
Copy link
Copy Markdown

@Yiwen-Gao Yiwen-Gao left a comment

Choose a reason for hiding this comment

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

There are some structs that we also duplicate in svm-rollup and apply serialization macros to. Do you have a system for which structs get modified in our Agave fork VS which are duplicated in svm-rollup?

@DudessaPr
Copy link
Copy Markdown
Author

There are some structs that we also duplicate in svm-rollup and apply serialization macros to. Do you have a system for which structs get modified in our Agave fork VS which are duplicated in svm-rollup?

For FeatureSetWrapper in SvmPrevConfig, we can use just FeatureSet and serialize it in agave now. Walter made that wrapper because we tried to not change Agave's code that time. About FeeRateGovernorWrapper, FeeRateGovernor has one field marked as serde-ignore that nullifies the value when the struct it cloned, and it is bad in our case when working with sovereign modules as they clone objects into sov-state. I do not really know what's the purpose of that serde-ignore therefore I created a Wrapper for that struct to be safe.

@Yiwen-Gao
Copy link
Copy Markdown

Got it, thanks for explaining. It sounds like we're going to modify the types in Agave directly moving forward. Feel free to make a follow-up PR to remove those types from svm-rollup and update Agave accordingly.

I agree that we should leave FeeRateGovernor alone and can leave that one type in svm-rollup.

@DudessaPr DudessaPr merged commit 7f22462 into feat/risc0 Jul 30, 2024
petarvujovic98 pushed a commit that referenced this pull request Oct 24, 2025
* Apply the retry code to the async pubsub client

Create a test server

```ts
import http from "http";

import { WebSocketServer } from "ws";

let attemptCount = 0;

const server = http.createServer();
const wss = new WebSocketServer({ noServer: true });

wss.on("connection", (ws) => {
  ws.send("Connection accepted.");
  ws.on("message", (msg) => console.log(`Received: ${msg}`));
});

server.on("upgrade", (req, socket, head) => {
  attemptCount += 1;

  if (attemptCount <= 4) {
    socket.write("HTTP/1.1 429 Too Many Requests\r\n\r\n");
    socket.destroy();
    console.log(`Rejected connection #${attemptCount} (429)`);
    return;
  }

  wss.handleUpgrade(req, socket, head, (ws) => {
    wss.emit("connection", ws, req);
    console.log("Connection accepted on attempt", attemptCount);
  });
});

server.listen(8080, () => {
  console.log("Server listening on port 8080");
});
```

Run `test_slot_subscription_async`:

```
Rejected connection #1 (429)
Rejected connection #2 (429)
Rejected connection #3 (429)
Rejected connection #4 (429)
Connection accepted on attempt 5
Received: {"id":1,"jsonrpc":"2.0","method":"slotSubscribe","params":[]}
```

* `s/async_with_retry/with_retry/`
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