Skip to content
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

Extend IApplication to generalize tBTC, RandomBeacon and TACo apps #156

Closed
cygnusv opened this issue Oct 23, 2023 · 2 comments · Fixed by #157
Closed

Extend IApplication to generalize tBTC, RandomBeacon and TACo apps #156

cygnusv opened this issue Oct 23, 2023 · 2 comments · Fixed by #157

Comments

@cygnusv
Copy link
Member

cygnusv commented Oct 23, 2023

In addition to the discussion on #88, while developing the TACo app contract, and particularly when integrating it with the dashboard, we've detected several methods that should be generalized in IApplication due to common workflows. This is specially relevant in contexts that consume the 3 contracts (i.e. the dashboard).

Also, consider if it's necessary the creation of an additional interface for Threshold apps that assume the existence of an operator role, something like IApplicationWithOperator, so we can abstract here methods like registerOperator().

@theref
Copy link
Member

theref commented Oct 23, 2023

When bonding an operator, Keep do:

function registerOperator(Data storage self, address operator) public {
        address stakingProvider = msg.sender;

a) it's called registerOperator
b) it only takes the operator as a parameter, msg.sender must be the associated stakingProvider

For Nucypher:

function bondOperator(
        address _stakingProvider,
        address _operator
    ) external onlyOwnerOrStakingProvider(_stakingProvider) 

we have the idea of an Owner as well as stakingProvider so it's a bit more complicated.

It would be ideal if we could bring this inline with Keep, certain areas of the dashboard are built very tightly around their Application Interface https://github.com/threshold-network/token-dashboard/blob/8652343fd254666e04bc90c601e4452c06c3fc38/src/threshold-ts/applications/index.ts#L82

Is that interface I've linked what everything else is based on? I don't want us to end up with multiple versions

@lukasz-zimnoch
Copy link
Member

Looks like the IApplication interface in the dashboard went a little bit too far. I think we should extract Keep-specific parts to a sub-interface:

export interface IApplication {
    // Only methods from https://github.com/threshold-network/solidity-contracts/blob/main/contracts/staking/IApplication.sol
}

export interface IWalletRegistry extends IApplication {
    // Methods specific to https://github.com/keep-network/keep-core/blob/324f66fb3f1003f6cfeb7d4149ae3f1d902dba2e/solidity/ecdsa/contracts/WalletRegistry.sol#L37
}

This way the actual contracts structure is reflected and you can easily add:

interface ITaco extends IApplication {
    // Whatever you need.
}

cc @michalsmiarowski

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants