Skip to content

simple partial-verifier#62

Merged
CluEleSsUK merged 32 commits into
mainfrom
feat/onlyswaps-verifier-v1
Sep 5, 2025
Merged

simple partial-verifier#62
CluEleSsUK merged 32 commits into
mainfrom
feat/onlyswaps-verifier-v1

Conversation

@CluEleSsUK

@CluEleSsUK CluEleSsUK commented Aug 22, 2025

Copy link
Copy Markdown
Contributor
  • polls all chains every 5 seconds
  • checks outstanding verifications from destination chain against source chain
  • writes partial signatures to libp2p
  • aggregate signatures are written back to source chain
  • configuration managed by toml/json file

caveats

  • not listening to block/fulfillment events, just polling the chains
  • there are a few ugly unwraps in the loops
  • configuration must be done by configuration file
  • some of the logic extraction hasn't been applied to other modules
  • shitty println! instead of using tracing

- calculates pending verifications on startup
- fetches the state on both source and destination chains
- verifies the parameters and 'signs' if they're correct
- created a new module for signing-related configuration
- parse signing/group configuration from local files
- start an in-memory event bus
- fetch pending verifications
- print a signature over them
@CluEleSsUK CluEleSsUK force-pushed the feat/onlyswaps-verifier-v1 branch from 3c66875 to 58c4af4 Compare August 27, 2025 13:29
@CluEleSsUK CluEleSsUK force-pushed the feat/onlyswaps-verifier-v1 branch from 1425328 to 79344b6 Compare August 27, 2025 14:11
@CluEleSsUK CluEleSsUK force-pushed the feat/onlyswaps-verifier-v1 branch from 79344b6 to 268357d Compare August 27, 2025 14:26
@CluEleSsUK CluEleSsUK marked this pull request as ready for review September 1, 2025 15:35
@CluEleSsUK CluEleSsUK requested a review from azixus September 1, 2025 15:39

@AnomalRoil AnomalRoil left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Half-way through, but here's a first batch of comments before lunch

Comment thread crates/config/src/signing.rs
Comment thread crates/config/src/signing.rs
Comment thread crates/config/src/signing.rs Outdated
Comment thread crates/config/src/signing.rs Outdated
Comment thread onlyswaps-verifier/Dockerfile
Comment thread onlyswaps-verifier/local-verifier-config.json
Comment thread onlyswaps-verifier/src/eth.rs
Comment thread onlyswaps-verifier/src/eth.rs
- added a README for the onlyswaps verifier
- updated the submodule name from onlysubs to onlyswaps
- renamed group to committee and some other notions around it
- combined keyshare and committee  configuration

@azixus azixus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nearly done, still need to go through signing.rs & pending.rs

Comment thread crates/config/Cargo.toml Outdated
Comment thread crates/utils/Cargo.toml
Comment thread onlyswaps-verifier/Cargo.toml Outdated
Comment thread onlyswaps-verifier/Cargo.toml Outdated
Comment thread onlyswaps-verifier/Cargo.toml Outdated
Comment thread onlyswaps-verifier/src/eth.rs
Comment thread onlyswaps-verifier/src/main.rs Outdated
Comment thread onlyswaps-verifier/src/main.rs Outdated
Comment thread onlyswaps-verifier/src/main.rs Outdated
Comment thread onlyswaps-verifier/src/threshold.rs
Comment thread .github/workflows/rust-build-and-tests.yml Outdated
Comment thread crates/config/src/signing.rs Outdated
Comment thread onlyswaps-verifier/README.md Outdated
Comment thread onlyswaps-verifier/README.md
t = 2 # the honest threshold required for reconstruction. This is *different* to the malicious threshold output by the [ADKG CLI](../crates/adkg-cli)
n = 3 # the total count of members in the committee, including yourself.

# `committee.nodes` should contain as many entries as `n` above, and should contain one for your own node.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

That's a weird requirement. I'd expect our own node identity to be defined elsewhere

Do we check the public key corresponds to the secret key above?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

it's this way because otherwise we have to rely on:

  • operators removing their own entry in the group file
  • allowing indexes being 'missing' at the end

we don't presently verify that the public key corresponds to the secret key; ...truth be told I'm not even sure this is even corresponding to the keyshare and not their own BN254 key

Comment thread onlyswaps-verifier/README.md Outdated
Comment thread onlyswaps-verifier/src/signing.rs
Comment thread onlyswaps-verifier/src/signing.rs
Comment thread onlyswaps-verifier/src/parsing.rs
Comment on lines +19 to +22
pub fn reconcile_transfer_params(
src_params: SwapRequestParameters,
dest_receipt: TransferReceipt,
) -> anyhow::Result<SwapRequestParameters> {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why are we not checking anything wrt to the request_id here?
Same about the nonce field?

How comes the latter is not verified to be unique / not have been seen before in here? Also, what is its purpose vs the request_id in general?

I guess the underlying question is: what prevents a replay here?

E.g. the same request is made again by the user, for the same amounts same source and destination, and all, but with a new nonce and a new request ID, obviously, and the last solver replays their first solve, without having solved the new request

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

requestID embeds a bunch of params about the tx, most notably the chainID; nonces are governed by the smart contract, so we can't reuse a (chainID, nonce) combo.

See: https://github.com/randa-mu/onlysubs-solidity/blob/c57c6e0406ea66b5510c4822bd52d505e6cee0e2/src/Router.sol#L276

In theory, we might reuse a nonce if we deploy multiple Router contracts with the same committee - we could consider embedding the Router contract address to stop that if you think it's necessary. cc @najienka

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Damn, beat me to it by 2 days :p

@CluEleSsUK CluEleSsUK force-pushed the feat/onlyswaps-verifier-v1 branch from 4d27c1d to 035a55f Compare September 3, 2025 14:58
Comment on lines +19 to +22
pub fn reconcile_transfer_params(
src_params: SwapRequestParameters,
dest_receipt: TransferReceipt,
) -> anyhow::Result<SwapRequestParameters> {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Damn, beat me to it by 2 days :p

@CluEleSsUK CluEleSsUK merged commit f7e20d6 into main Sep 5, 2025
5 of 6 checks passed
@CluEleSsUK CluEleSsUK deleted the feat/onlyswaps-verifier-v1 branch September 5, 2025 15:31
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