Adding interface for applications that use operator role and delay in… #157
Conversation
|
Solidity API documentation preview available in the artifacts of the https://github.com/threshold-network/solidity-contracts/actions/runs/6828081757 check. |
vzotova
force-pushed
the
app-interfaces
branch
from
845232a
to
81dd1eb
Compare
|
Solidity API documentation preview available in the artifacts of the https://github.com/threshold-network/solidity-contracts/actions/runs/6828164286 check. |
| // TODO consider that? | ||
| // /// @notice Used by additional role (owner for example) to set operator address that will | ||
| // /// operate a node for the specified staking provider. | ||
| // function registerOperator(address stakingProvider, address operator) external; |
There was a problem hiding this comment.
What's your opinion on this @vzotova @lukasz-zimnoch ? My suggestion would be to not include it in the interface, so it can be optionally implemented by applications that allow owners to set operators.
There was a problem hiding this comment.
I'm good with both options
There was a problem hiding this comment.
What's your opinion on this @vzotova @lukasz-zimnoch ? My suggestion would be to not include it in the interface, so it can be optionally implemented by applications that allow owners to set operators.
Agreed! That feels optional and should not live on the interface level.
There was a problem hiding this comment.
Then we need to remove it here as well? https://github.com/threshold-network/token-dashboard/blob/bbdc89047f2f32905cf35ccbb07ac414dc9b0ab2/src/threshold-ts/applications/index.ts#L191
We should probably be using the solidity-contracts repo in the dashboard rather than having a bunch of interfaces hard coded there
There was a problem hiding this comment.
ok, removed
@theref there is "regular" registerOperator in your link and this method still in interface
There was a problem hiding this comment.
oh yes sorry, hadn't noticed the difference in signature.
ok, well the dashboard is using an interface to specify how application contracts should behave. If we don't include the version that allows applications to set Operators then it's not going to be available on the dashboard (unless we completely change the dashboard architecture)
| external | ||
| view | ||
| returns ( | ||
| uint96 _minimumAuthorization, |
There was a problem hiding this comment.
Why the _ here? Same for _stakingProvider in L51.
There was a problem hiding this comment.
otherwise it shadows function minimumAuthorization
|
Solidity API documentation preview available in the artifacts of the https://github.com/threshold-network/solidity-contracts/actions/runs/6864825267 check. |
vzotova
force-pushed
the
app-interfaces
branch
from
9704156
to
ea5e4e2
Compare
|
Solidity API documentation preview available in the artifacts of the https://github.com/threshold-network/solidity-contracts/actions/runs/6864969050 check. |
|
Solidity API documentation preview available in the artifacts of the https://github.com/threshold-network/solidity-contracts/actions/runs/6895222325 check. |
vzotova
force-pushed
the
app-interfaces
branch
from
ccf0e78
to
93d5b92
Compare
|
Solidity API documentation preview available in the artifacts of the https://github.com/threshold-network/solidity-contracts/actions/runs/6931805960 check. |
|
Solidity API documentation preview available in the artifacts of the https://github.com/threshold-network/solidity-contracts/actions/runs/6931814522 check. |
vzotova
force-pushed
the
app-interfaces
branch
from
57889e2
to
db9eccb
Compare
|
Solidity API documentation preview available in the artifacts of the https://github.com/threshold-network/solidity-contracts/actions/runs/6931841466 check. |
vzotova
force-pushed
the
app-interfaces
branch
from
db9eccb
to
4450472
Compare
|
Solidity API documentation preview available in the artifacts of the https://github.com/threshold-network/solidity-contracts/actions/runs/6931919073 check. |
cygnusv
force-pushed
the
app-interfaces
branch
from
4450472
to
db87adc
Compare
… authorization decrease
Co-authored-by: David Núñez <[email protected]> Co-authored-by: Derek Pierre <[email protected]>
vzotova
force-pushed
the
app-interfaces
branch
from
db87adc
to
9b61552
Compare
… authorization decrease.
Fixes #156
Main suggestion here is to use both interfaces for all apps: TBCv2, RandomBeacon, TACo.
Names for method inherit those from
RandomBeaconandTBTCv2, except one additional method to register operator using additional role (please leave comment: remove or keep it).That unification will help with tracking changes between TBTC and TACo contracts, simplify dashboard interaction with contracts and also help with contract pool developing in the future.