-
Notifications
You must be signed in to change notification settings - Fork 11
feat: stash kubernetes release #613
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
chore: move non sensitive data out of sops files
…develop-alignment
…ignment chore: post ferries release alignment
* revert keys changes * fetch logs in batches * chore: revert rates key optimisation Signed-off-by: Olga Konstantinov <[email protected]> * chrore: Fix race condition of finalized and current block. Add debug trace * feat: endpoint for garbage collection --------- Signed-off-by: Olga Konstantinov <[email protected]> Co-authored-by: Olga Konstantinov <[email protected]> Co-authored-by: Gonzalo DC <[email protected]>
* chore: fix missing metrics on startup * chrore: format with go fmnt and added github.com/btcsuite/btcd v0.22.0-beta to fix ambiguous import --------- Co-authored-by: mateuszaaa <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @mateuszaaa, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request delivers a substantial infrastructure overhaul for the stash
service. Key changes include upgrading the Node.js runtime, consolidating Redis instances for unified data management, and streamlining the gasp-avs
Docker image by removing external binary dependencies. These updates aim to enhance performance, simplify deployment, and improve overall system maintainability.
Highlights
- Node.js Upgrade: The project's Node.js runtime has been upgraded from version 18.20.7 to 22.14.0, accompanied by comprehensive dependency updates and linter fixes across the codebase.
- Redis Consolidation: The application now utilizes a single Redis instance for all data storage, including timeseries data, replacing the previous setup that used separate Redis instances. This simplifies the architecture and configuration.
- GASP AVS Docker Image Refactor: The
gasp-avs
Docker image has undergone a significant refactoring, removing its dependency on thegasp-node
binary and the supervisor setup. Thegasp-avs
service now runs as a single, streamlined process within its container. - CI/CD Pipeline Simplification: The image building and pushing processes within the CI/CD pipeline have been simplified, reflecting the consolidated Redis setup and the refactored
gasp-avs
Docker image. - Data Management Improvement: A new endpoint has been introduced for garbage collecting old pool rates data, which helps in maintaining data hygiene and optimizing storage.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a significant set of changes, including a Node.js version upgrade, dependency cleanup, and architectural improvements like decoupling the gasp-avs
service and consolidating Redis usage. These are all positive steps towards a more maintainable and streamlined system.
My main concern is the number of skipped tests across various files. While the moon.yml
configuration re-enables the test command, many test suites and individual tests are still disabled within the test files themselves. It's critical to address this to ensure regressions are not introduced. I've left specific comments on each skipped test.
I also found a leftover console.log
statement that should be removed. Otherwise, the code changes look consistent and well-aligned with the goals described in the commit messages.
@@ -8,7 +9,7 @@ let transactionData: any | |||
const wrongType = 'deposittt' | |||
const type = 'deposit' | |||
|
|||
describe('TracingController', () => { | |||
describe.skip('TracingController', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
||
function validateValidApyRange(body) { | ||
const apy = parseFloat(body.apy) | ||
expect(apy).to.be.lessThanOrEqual(300) | ||
expect(apy).to.be.greaterThan(100) | ||
} | ||
|
||
describe('APi tests: Collator apy - dailyRewards', () => { | ||
describe.skip('APi tests: Collator apy - dailyRewards', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -65,7 +66,7 @@ describe('APi tests: token stats', () => { | |||
expect(onlyGaspV2[0]).toEqual(gaspV2Token) | |||
}) | |||
}) | |||
it('GET /token/list/stats - List matches with all the tokens with pool', async () => { | |||
it.skip('GET /token/list/stats - List matches with all the tokens with pool', async () => { //to fix data on the frontend node |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
import { tokenIDs } from './utils' | ||
|
||
describe('/GET tvl-history', () => { | ||
it('GET /tvl-history - Schema validation', async () => { | ||
it.skip('GET /tvl-history - Schema validation', async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
||
vi.mock('gasp-sdk') | ||
|
||
describe('[CoinGecko listing]', () => { | ||
describe.skip('[CoinGecko listing]', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
||
describe('L1LogScraper', () => { | ||
describe.skip('L1LogScraper', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it.skip('should mock the 24 hours rewards history endpoint', async () => { | ||
//TODO: rewrite this test,data that we have in a db from last year so returning [] | ||
const expectedResponse: ResponseRewards[] = [ | ||
{ liquidityTokenId: '8', amountClaimed: '503619012693557062128' }, | ||
] | ||
const results = await rewards24hours( | ||
'5FFTemNzVqVduFk7n8z7G6qukrnBfTTQRE8EGPbjmtdpz2c1' | ||
'0x928f1040adb982d3ab32a62dc8eda57e9b81b4dd', | ||
) | ||
expect(results).deep.equal(expectedResponse) | ||
}) | ||
|
||
it('should mock the month rewards history endpoint', async () => { | ||
it.skip('should mock the month rewards history endpoint', async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
import Decimal from 'decimal.js' | ||
import IORedis from 'ioredis' | ||
import { GenericContainer, Wait } from 'testcontainers' | ||
import { afterAll, beforeAll, chai, describe, it, vi } from 'vitest' | ||
chai.should() | ||
|
||
describe.skip('Integration Test using Test Containers', function () { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
||
vi.mock('../src/repository/TransactionRepository') | ||
vi.mock('../src/util/Logger') | ||
|
||
describe('TracingService', () => { | ||
describe.skip('TracingService', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -210,12 +210,14 @@ func (agg *Aggregator) Start(ctx context.Context) error { | |||
if agg.rpcServer != nil { | |||
go agg.rpcServer.startServer(ctx, agg.apiKey, runTriggerC) | |||
} | |||
|
|||
|
|||
recordMetrics(agg.logger, agg.ethRpc) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
…yment feature: Disable automatic deployment on merge to main
Run report for b407f898
|
Action | Time | Status | Info | |
---|---|---|---|---|
🟩 | SyncWorkspace |
368.2ms | Passed | |
⬛️ | SetupToolchain(system) |
0.2ms | Skipped | |
🟩 | SyncProject(system, sequencer) |
1.4ms | Passed | |
🟩 | SyncProject(system, gasp-avs) |
1.7ms | Passed | |
🟩 | SyncProject(system, contracts) |
1.3ms | Passed | |
🟩 | SyncProject(system, updater) |
2.8ms | Passed | |
🟩 | SyncProject(system, gasp-node) |
4.1ms | Passed | |
🟩 | SyncProject(system, avs-aggregator) |
0.5ms | Passed | |
🟩 | SetupToolchain(node:22.14.0) |
5.6s | Passed | |
🟩 | SetupToolchain(node:18.20.7) |
8.7s | Passed | |
🟩 | RunTask(contracts:buildkit-build-and-push-image-digests) |
21.3s | Passed | |
🟩 | RunTask(sequencer:buildkit-build-and-push-image-digests) |
3m 49s | Passed | SLOW |
🟩 | RunTask(updater:buildkit-build-and-push-image-digests) |
4m 16s | Passed | SLOW |
🟩 | RunTask(gasp-avs:buildkit-build-and-push-image-digests) |
5m 14s | Passed | SLOW |
🟩 | SyncProject(node, stash) |
0.9ms | Passed | |
🟩 | SyncProject(node, ferry-deposit) |
0.5ms | Passed | |
🟩 | SyncProject(node, ferry-withdrawal) |
0.4ms | Passed | |
🟩 | RunTask(contracts:build-image-ci) |
9.5s | Passed | |
🟩 | RunTask(sequencer:build-image-ci) |
15.2s | Passed | |
🟩 | RunTask(updater:build-image-ci) |
12.8s | Passed | |
And 12 more... |
Expanded report
Action | Time | Status | Info | |
---|---|---|---|---|
🟩 | RunTask(gasp-avs:build-image-ci) |
12.9s | Passed | |
🟩 | RunTask(avs-aggregator:buildkit-build-and-push-image-digests) |
1m 44s | Passed | |
🟩 | RunTask(stash:buildkit-build-and-push-image-digests) |
1m 57s | Passed | |
🟩 | RunTask(ferry-deposit:buildkit-build-and-push-image-digests) |
2m 16s | Passed | SLOW |
🟩 | RunTask(avs-aggregator:build-image-ci) |
15.2s | Passed | |
🟩 | RunTask(stash:build-image-ci) |
15.7s | Passed | |
🟩 | RunTask(ferry-deposit:build-image-ci) |
16.9s | Passed | |
🟩 | RunTask(gasp-node:buildkit-build-and-push-image-digests-fast-runtime) |
8m 58s | Passed | SLOW |
🟩 | RunTask(ferry-withdrawal:buildkit-build-and-push-image-digests) |
1m 41s | Passed | |
🟩 | RunTask(ferry-withdrawal:build-image-ci) |
16.8s | Passed | |
🟩 | RunTask(gasp-node:buildkit-build-and-push-image-digests-standard-runtime) |
6m 47s | Passed | SLOW |
🟩 | RunTask(gasp-node:build-image-ci) |
23.5s | Passed |
Environment
OS: Linux
Matrix:
prefix = build-docker-images
name = Build docker images
runner = ubuntu-24.04
buildkit-enable = true
buildkit-replicas = 5
command = moon :build-image-ci
Variables:
MOON_COLOR = 2
MOON_DEBUG_PROTO_INSTALL = false
MOON_CACHE = off
MOON_VERSION = 1.34.0
Touched files
.github/workflows/deploy-stash.yml
moon
cli to1.34.0
version to be used within gha workflowsstash