Skip to content

Comments

Initial Handshake Implementation#6

Open
AcoSmrkas wants to merge 10 commits intorosen-bridge:devfrom
AcoSmrkas:handshake
Open

Initial Handshake Implementation#6
AcoSmrkas wants to merge 10 commits intorosen-bridge:devfrom
AcoSmrkas:handshake

Conversation

@AcoSmrkas
Copy link

Implementation following RCS-003 - https://github.com/rosen-bridge/rcs/tree/master/rcs-003

Comment on lines +1 to +15
# Changelog

All notable changes to this project will be documented in this file.

The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [Unreleased]

## [0.1.0] - 2025-10-16

### Added

- Initial release of Handshake RPC observation extractor
- Implemented HandshakeRpcObservationExtractor class
Copy link
Member

Choose a reason for hiding this comment

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

You're not supposed to add the changelog. The changelog is auto-generated using the changeset files.

@@ -0,0 +1,46 @@
{
"name": "@rosen-bridge/handshake-rpc-observation-extractor",
"version": "0.1.0",
Copy link
Member

Choose a reason for hiding this comment

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

The version should start with 0.0.0. It will be updated automatically after applying changesets before the release.

@@ -0,0 +1,46 @@
{
"name": "@rosen-bridge/handshake-rpc-observation-extractor",
Copy link
Member

Choose a reason for hiding this comment

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

The naming convention for observation-extractors doesn't include the network. The package name should be handshake-observation-extractor; if any network is added for HNS in the next steps, it will be implemented in this package.
Please fix the ReadMe names too.

Comment on lines +1 to +18
import { defineConfig } from 'vitest/config';

export default defineConfig({
test: {
globals: true,
coverage: {
all: true,
provider: 'istanbul',
reporter: 'cobertura',
},
passWithNoTests: true,
poolOptions: {
forks: {
singleFork: true,
},
},
},
});
Copy link
Member

Choose a reason for hiding this comment

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

Create the vitest config from the shared available configuration:

Suggested change
import { defineConfig } from 'vitest/config';
export default defineConfig({
test: {
globals: true,
coverage: {
all: true,
provider: 'istanbul',
reporter: 'cobertura',
},
passWithNoTests: true,
poolOptions: {
forks: {
singleFork: true,
},
},
},
});
import { defineProject, mergeConfig } from 'vitest/config';
import configShared from '../../vitest.shared';
export default mergeConfig(configShared, defineProject({}));

Comment on lines +111 to +116
// Filter out name auction transactions (covenant type != 0)
// Bridge only accepts transactions where all outputs have covenant type 0 (NONE)
const filteredTxs = blockTxs.filter((tx) => {
return !tx.vout.some((output) => output.covenant.type !== 0);
});

Copy link
Member

Choose a reason for hiding this comment

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

Filtering is correct, but the scanner is not responsible for filtering transactions. The filter should be implemented in the extractor. You can implement the preprocessTransactions in the extractor to do so.

@@ -0,0 +1,43 @@
{
"name": "@rosen-bridge/handshake-rpc-scanner",
"version": "0.1.0",
Copy link
Member

Choose a reason for hiding this comment

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

0.0.0

@@ -0,0 +1,27 @@
/* eslint-disable @typescript-eslint/no-explicit-any */
Copy link
Member

Choose a reason for hiding this comment

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

Better to turn it off for the specific line only.

"devDependencies": {
"@vitest/coverage-istanbul": "^3.1.4",
"tsx": "^4.19.4",
"vitest": "^3.1.4"
Copy link
Member

Choose a reason for hiding this comment

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

tsx and vitest installed in the root package.json should not exist here too.

"vitest": "^3.1.4"
},
"engines": {
"node": ">=22.18.0"
Copy link
Member

Choose a reason for hiding this comment

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

npm version

Comment on lines +1 to +18
import { defineConfig } from 'vitest/config';

export default defineConfig({
test: {
globals: true,
coverage: {
all: true,
provider: 'istanbul',
reporter: 'cobertura',
},
passWithNoTests: true,
poolOptions: {
forks: {
singleFork: true,
},
},
},
});
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
import { defineConfig } from 'vitest/config';
export default defineConfig({
test: {
globals: true,
coverage: {
all: true,
provider: 'istanbul',
reporter: 'cobertura',
},
passWithNoTests: true,
poolOptions: {
forks: {
singleFork: true,
},
},
},
});
import { defineProject, mergeConfig } from 'vitest/config';
import configShared from '../../vitest.shared';
export default mergeConfig(configShared, defineProject({}));

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.

2 participants