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

Stop using CheckBridge ABCI #410

Open
troggy opened this issue Jan 31, 2020 · 5 comments
Open

Stop using CheckBridge ABCI #410

troggy opened this issue Jan 31, 2020 · 5 comments

Comments

@troggy
Copy link
Member

troggy commented Jan 31, 2020

Bounty

We are using patched Tendermint which is not a great practice. This bounty is a second take to propose a solution to get rid of custom Tendermint and use vanilla one.

Original discussion: #357

Scope

  • change bridgeState to keep multiple period proposals. Right now it is just a single periodProposal object for the last complete period and stalePeriodProposal object for the older period if it is not on the root chain yet. Proposing to replace both with an map of proposals:
     type BlocksRootType = string;
    
     type PeriodProposal = {
       height: number;
       proposerSlotId: number;
       votes: number[]; 
       blocksRoot: string;
       periodRoot: string;
       prevPeriodRoot: string;
       updatedAt: timestamp;
     };
    
     periodProposals: Record<BlocksRootType, PeriodProposal>;
    Calculate periodRoot for proposal using const { periodRoot } = period.periodData(); (make sure period has validator data set).
  • every time we create a new period proposal (see startNewPeriod.js) it is just added to the map. Store into local db and read it back on node start.
  • every time the node got period Submission event and saved it to local db, remove corresponding proposal record from the map
  • change submitPeriod to update updatedAt attribute of the proposal to the current timestamp. Make sure the change is saved to local db.
  • every 16th block check all the proposals and apply the logic from checkBridge to those with updatedAt older than 2 minutes (TODO: is it a good gap time?).
  • remove checkBridge handler (/src/period/index.js)
  • remove custom tendermint — stick to vanilla one of the same version we had (tendermint upgrade is out of scope)

Deliverables

  • updated code
  • updated tests
  • writeup on manual integration tests performed

Gain for the project

reduce complexity, easier to upgrade tendermint, no consensus stops

Expert

TBD

Publicly visible delivery

Roles

bounty gardener: @troggy / 100 DAI
bounty worker: name / share
bounty reviewer: name / share

Funded by

Dev Circle

@troggy
Copy link
Member Author

troggy commented Jan 31, 2020

@johannbarbie @TheReturnOfJan @pinkiebell

What are the implications of the setup where we are not stopping consensus in case of delayed period submission? Unsubmitted periods will be piling up, exits for younger utxos are not possible, but eventually all the periods should make it to the root chain in PoS setup as there is an incentive for other validators to submit stalling periods. Overall, the situation is not any different from the situation when we stop the consensus.

@pinkiebell
Copy link
Contributor

As far as I know, the bridge contracts allows periods to be submitted only after each passing epoch. So, the bridge state would fall behind for each (missed) period? This may not be a problem actually, but a hardlimit should be implemented somewhere.

Imagine, if the tendermint network computes new blocks very fast, faster than period submissions are allowed to happen, then that could be a problem for chain security, no?

🤔

@TheReturnOfJan
Copy link

TheReturnOfJan commented Feb 3, 2020

As @pinkiebell said, I think the main problem is if 32*(blocks/second) produced by tendermint is consistently bigger than periods/second swallowed by ethereum mainnet. In that case the non-finalised tip of the tendermint chain will only grow over time (the funds in blocks not commited to main net cannot be exited).
I was not able to parse your proposal fully, does it address this problem?

@troggy
Copy link
Member Author

troggy commented Feb 3, 2020

So, the bridge state would fall behind for each (missed) period?

Current implementation of contract doesn't allow missing periods — they chained to each other, and bridge will reject submission if the previous period is on submitted.

that could be a problem for chain security, no?
does it address this problem?

proposal is not addressing this, but, probably, the current implementation doesn't really address it as well: both stopped chain and not-checkpoint chain are unusable. Though, I admit, that running chain without checkpoints will be perceived as "safe".

@pinkiebell @TheReturnOfJan thanks for the feedback 🙏I guess you are right, it is not safe in general.

@troggy
Copy link
Member Author

troggy commented Feb 3, 2020

What if we introduce block time to the leap chain? E.g. block time of 3 seconds will create a 1.5 minute gap between periods.

Though, anyway, in case of validator failure to submit period the chain will run unsecured.

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

No branches or pull requests

3 participants