App <-> bridge #98
App <-> bridge #98
Conversation
vzotova
force-pushed
the
cbd-pre-app
branch
2 times, most recently
from
96d0926
to
335494d
Compare
vzotova
requested review from
KPrasch,
arjunhassard,
cygnusv,
theref,
derekpierre and
manumonti
| /** | ||
| * @dev Checks caller is source of data | ||
| */ | ||
| modifier onlySource() |
| assert len(events) == 1 | ||
| event = events[0] | ||
| assert event["stakingProvider"] == staking_provider | ||
| assert event["fromAmount"] == value // 2 | ||
| assert event["toAmount"] == value | ||
|
|
||
| # Increase again witjout syncing with StakeInfo |
There was a problem hiding this comment.
| # Increase again witjout syncing with StakeInfo | |
| # Increase again without syncing with StakeInfo |
|
|
||
| def test_authorization_increase(accounts, threshold_staking, pre_application): | ||
|
|
||
| def test_authorization_increase(accounts, threshold_staking, pre_cbd_application, stake_info): |
There was a problem hiding this comment.
Do we want to just call pre_cbd_application -> taco_application? This and other test files.
|
|
||
|
|
||
| contract PolygonRoot is FxBaseRootTunnel { | ||
| contract PolygonRoot is FxBaseRootTunnel, IUpdatableStakeInfo { |
There was a problem hiding this comment.
Great idea to implement IUpdatableStakeInfo
| require(address(_updatableStakeInfo) != address(updatableStakeInfo), "New address must not be equal to the current one"); | ||
| if (address(_updatableStakeInfo) != address(0)) { | ||
| // trying to call contract to be sure that is correct address | ||
| _updatableStakeInfo.updateOperator(address(0), address(0)); |
There was a problem hiding this comment.
So this is to implicitly check the source in PolygonRoot, right?
There was a problem hiding this comment.
and to check that contract is correct one in general
| * @notice Set contract for multi-chain interactions | ||
| */ | ||
| function setUpdatableStakeInfo(IUpdatableStakeInfo _updatableStakeInfo) external onlyOwner { | ||
| require(address(_updatableStakeInfo) != address(updatableStakeInfo), "New address must not be equal to the current one"); |
There was a problem hiding this comment.
Do we really need this check? What's the worst can happen?
There was a problem hiding this comment.
considering that is method for DAO - I prefer to more checks than less in any case
|
|
||
|
|
||
| /** | ||
| * @title Adjudicator | ||
| * @notice Supervises operators' behavior and punishes when something's wrong. | ||
| * @dev |v3.1.1| | ||
| */ | ||
| abstract contract Adjudicator { | ||
| contract Adjudicator { |
There was a problem hiding this comment.
It seems Adjudicator is not upgradeable anymore, right? In that case, the uint256[50] private reservedSlots; in L46 should be removed.
There was a problem hiding this comment.
Honestly, Adjudicator needs reincarnation but I guess it's not time. So I'll remove those fields but ideally make it upgradeable
There was a problem hiding this comment.
Since we have a setAdjudicator() method in the app, do we really need it to be upgradeable? We can just deploy a new version and set it.
There was a problem hiding this comment.
Adjudicator has some state that ideally to preserve. For example, history to not slash twice for the same misbehavior. That's main reason to make it upreadable with the current design
… to set in app, restricts access to update PolygonRoot
…s bug with releasing operator
…perator with StakeInfo
Unifies communication between app and bridge(s) through interface. App can work without bridge, with
StakeInfoin the same network, with bridge. Besides it's possible to implement manager of bridges without changing app.TACoApplication+ methods to call interfacePolygonRoot