feat: tee solver registry, vault and server#1
Conversation
| AddLiquidity { | ||
| pool_id: &'a AccountId, | ||
| account_id: &'a AccountId, | ||
| amounts: &'a Vec<U128>, | ||
| }, | ||
| RemoveLiquidity { | ||
| pool_id: &'a AccountId, | ||
| account_id: &'a AccountId, | ||
| amounts: &'a Vec<U128>, | ||
| }, |
There was a problem hiding this comment.
I don't think these two are needed. The flow is basically that once a token graduates from the launchpad, it calls create_liquidity_pool with the tokens left in the launchpad
There was a problem hiding this comment.
yes. As discussed, this will be considered for next milestones. We'll skip adding/removing liquidity features in this PR.
|
|
||
| log!("verify result: {:?}", result); | ||
|
|
||
| // TODO: verify predecessor implicit account is derived from this public key |
There was a problem hiding this comment.
you also need to check that this key is generated from within the TEE. This should be part of the quote verification. The RTMR3 hash should be extended with the public key, i.e, rtmr3 = hash(rmtr3 || public_key)
There was a problem hiding this comment.
This has been implemented by adding the derived public key as the raw report data when generating the quote hex.
There was a problem hiding this comment.
Please remove this comment then
There was a problem hiding this comment.
This TODO is to verify the predecessor account's address (implicit account) matches with the public key. This can be optional.
Let me confirm and may add this enhancement later.
|
@bowenwang1996 Hey Bowen, this PR is not perfect and has some limitations that we need to enhance as I mentioned in the PR's description, but it's ready for review. Please invite any reviewer who needs to look into this system. |
|
|
||
| export const ENV = requiredEnv('ENV') as Env; | ||
|
|
||
| export function requiredEnv(name: string): string { |
There was a problem hiding this comment.
You can use Zod and simplify this and index.ts by quite a bit. No need for custom env management/validation
| @@ -0,0 +1,60 @@ | |||
| import { getConfig } from '../config'; | |||
| import { connect, KeyPair, keyStores, Near } from 'near-api-js'; | |||
There was a problem hiding this comment.
Don't use near-api-js, that's actually deprecated. Use @near-js/account, @near-js/client, or @near-js/providers. It includes all the functions you've written here.
In fact, you might not even need this utils file at all
There was a problem hiding this comment.
thanks. We can refactor with either near-api-js v6.0.0+ or @near-js/* packages later in another PR.
I'll keep the utils in this PR.
There was a problem hiding this comment.
near-api-js as a package is deprecated. v6 recommended moving to @near-js. It's all part of the same monorepo, which can be a bit confusing
| import axios, { AxiosInstance, AxiosRequestConfig } from 'axios'; | ||
| import { getApiKey } from '../utils/credentials'; | ||
| import { logger } from '../../utils/logger'; |
There was a problem hiding this comment.
Why vendor this library? You can npm install it and import the APIClient instead.
It'll be a lot to manually maintain this and keep it synced with https://github.com/Phala-Network/phala-cloud-cli
There was a problem hiding this comment.
Ideally yes, but I think ApiClient is not exported in the phala package.
How do you think we can import ApiClient?
There was a problem hiding this comment.
They do export it here: https://github.com/Phala-Network/phala-cloud-cli/blob/main/src/api/client.ts#L18
However, I see that the package.json bundles the code, and installing from Git doesn't really help either, since it's expecting built-files only. Another option would be to fork it to support importing it as a library.
There was a problem hiding this comment.
- [Feature] Expose src/api as an importable library module Phala-Network/phala-cloud-cli#33
- feat: Expose src/api as an importable library module Phala-Network/phala-cloud-cli#34
Just opened an issue in the repo and a corresponding PR to fix that. If/once they merge, then we no longer need to vendor this library 😄
| } | ||
|
|
||
| public async getWorkerLen(): Promise<number> { | ||
| await this.init(); |
There was a problem hiding this comment.
No need for multiple inits - you can move this to a constructor, or if you'd prefer, just make everything static methods and keep the inits.
There was a problem hiding this comment.
That was supposed to use constructor in the initial implementation, but constructor function cannot be async. We can refactor this later.
There was a problem hiding this comment.
So it looks like:
init() calls getConfig() which is async
getConfig() dynamically imports either mainnet.ts or testnet.ts based on the ENV variable
But ENV is determined at module load time from process.env.ENV
constructor() {
// This could be done synchronously since ENV is available immediately
const config = ENV === 'mainnet' ? mainnetConfig : testnetConfig;
this.solverRegistryContract = config.near.contract.solverRegistry;
}Also dynamically importing modules is not really ideal.
There was a problem hiding this comment.
I agree. make sense. will update this later.
| { | ||
| "compilerOptions": { | ||
| "target": "ES2020", | ||
| "module": "commonjs", |
There was a problem hiding this comment.
No need for commonjs support, that's a legacy setting
| "start": "node -r dotenv/config dist/index.js", | ||
| "dev": "ts-node -r dotenv/config src/index.ts", | ||
| "test": "jest", | ||
| "lint": "eslint . --ext .ts" |
There was a problem hiding this comment.
Check out biome for linting/formatting instead of eslint
There was a problem hiding this comment.
thanks. we may migrate to biome later.
| #[near(contract_state)] | ||
| pub struct Contract {} | ||
|
|
||
| /// TODO: the contract can be deployed as a global contract |
There was a problem hiding this comment.
Let's create an issue for this one
|
|
||
| log!("verify result: {:?}", result); | ||
|
|
||
| // TODO: verify predecessor implicit account is derived from this public key |
There was a problem hiding this comment.
Please remove this comment then
| // fn require_approved_codehash(&self) { | ||
| // let worker = self.get_worker(env::predecessor_account_id()); | ||
| // self.assert_approved_codehash(&worker.codehash); | ||
| // } |
There was a problem hiding this comment.
why is this commented out?
There was a problem hiding this comment.
This assertion function is used when the worker invokes some functions via the registry contract (e.g. get Chain Signatures of some intents via a sign_intents function), but in practice we didn't use this function yet. I prefer keep the commented code for now, and we can remove it later if there's no such use case.
| // `add_liquidity` and `remove_liquidity` are not needed for now | ||
| // #[payable] | ||
| // pub fn add_liquidity( | ||
| // &mut self, | ||
| // pool_id: u32, | ||
| // token_ids: Vec<AccountId>, | ||
| // amounts: Vec<Balance>, | ||
| // ) { | ||
| // require!(token_ids.len() == 2, "Must have exactly 2 tokens"); | ||
| // require!(amounts.len() == 2, "Must have exactly 2 amounts"); | ||
| // require!(amounts[0] > 0, "Amount must be greater than 0"); | ||
| // require!(amounts[1] > 0, "Amount must be greater than 0"); | ||
|
|
||
| // let pool = self.pools.get(pool_id).expect("Pool not found"); | ||
| // let shares_total_supply = pool.shares_total_supply; | ||
| // } | ||
|
|
||
| // #[payable] | ||
| // pub fn remove_liquidity(&mut self, pool_id: u32, shares: U128) { | ||
| // let shares = shares.0; | ||
| // require!(shares > 0, "Shares must be greater than 0"); | ||
|
|
||
| // let pool = self.pools.get(pool_id).expect("Pool not found"); | ||
| // // pool.shares_total_supply -= shares; | ||
| // } | ||
| } |
There was a problem hiding this comment.
Let's create an issue for allowing users to add liquidity
| "devDependencies": { | ||
| "@types/jest": "^29.5.0", | ||
| "@types/node": "^18.15.11", | ||
| "@typescript-eslint/eslint-plugin": "^5.57.1", |
There was a problem hiding this comment.
This is several major versions behind. The latest is 8.34.0
| "@types/node": "^18.15.11", | ||
| "@typescript-eslint/eslint-plugin": "^5.57.1", | ||
| "@typescript-eslint/parser": "^5.57.1", | ||
| "eslint": "^8.37.0", |
There was a problem hiding this comment.
This is a major version behind, the latest is 9.29.0
| }, | ||
| "devDependencies": { | ||
| "@types/jest": "^29.5.0", | ||
| "@types/node": "^18.15.11", |
| "jest": "^29.5.0", | ||
| "ts-jest": "^29.1.0", | ||
| "tsx": "^4.20.3", | ||
| "typescript": "^5.0.3" |
There was a problem hiding this comment.
This version of typescript is over a year old
There was a problem hiding this comment.
Thanks! I'll update the dependencies this week.
NEAR Intents TEE Solver Registry
The NEAR Intents TEE Solver Registry is a protocol that enables secure and private execution of NEAR Intents solvers using Trusted Execution Environment (TEE) technology. This project consists of smart contracts for managing solver registration and a server for launching and managing TEE solvers.
This protocol allows liquidity pools creation for NEAR Intents. Liquidity providers can transfer funds into the pools' smart contracts. Only the solvers who're running within TEE with the approved Docker images can be registered and authorized to operate against the pools' assets.
Components
Smart Contracts
solver-registry: Support liquidity pools creation. Manage registration and verification of TEE solvers for each liquidity pool.intents-vault: The vault contract that manage the pool's asset within NEAR Intents.Solver Management Server
TEE enabled AMM Solver
TODOs
add_public_keyfunction