-
Notifications
You must be signed in to change notification settings - Fork 276
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
Adds QGB related ADRs #250
Conversation
@evan-forbes Please take a look on this, when you have time, if it is in the right direction. And, if you have any idea what needs to be added to this :D Thanks a lot |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a great start! 👍 Originally I had only imagined a single ADR, but I like having three smaller ones. Left some comments
@evan-forbes Please take a look when you have time again |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thinking on this a bit more, we might just want to not include the overview ADR yet. We only need to add state to the app for the qgb when we have slashing, if we weren't planning on doing that, then we would do everything off chain. Unless we want to talk about that, I actually think its okay to get rid of the overview portion. Perhaps we can add in some grander overview later? over perhaps in the qgb repo. Still not sure yet.
The other two ADRs are good tho, and we should definitely include them
[`submitDataRootTupleRoot()`](https://github.com/celestiaorg/quantum-gravity-bridge/blob/980b9c68abc34b8d2e4d20ca644b8aa3025a239e/ethereum/solidity/src/QuantumGravityBridge.sol#L328). | ||
Each event is a batch of `DataRootTuples`, with each tuple representing a single data root (i.e. block header). | ||
Relayed tuples are in the same order as Celestia block headers. | ||
For more details, check the data commitment ADR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Link to ADR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't have an ADR in QGB, no ? I only found tracking issues. And these;
celestiaorg/blobstream-contracts#20
celestiaorg/blobstream-contracts#4
Do you mean one of them ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think John was referring simply to the other ADR included in this PR. We can just link to this that file in this PR I think
|
||
Finally, if there are no validator set updates for the unbonding window, the bridge must halt. | ||
|
||
### When are validator sets created |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to submit a "new" validator set within the unbonding window even if it hasn't changed? Otherwise the bridge contract doesn't know if the bridge should shut down due to no updates within the unbonding window, or if there simply were no updates.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understand right, https://github.com/SweeXordious/celestia-app/blob/e4760741315840395c589d44414a185c1f641084/x/qgb/abci.go#L42
we will always post valsets requests even without it being changed. @evan-forbes Correct me if I am wrong please
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to submit a "new" validator set within the unbonding window even if it hasn't changed? Otherwise the bridge contract doesn't know if the bridge should shut down due to no updates within the unbonding window, or if there simply were no updates.
it looks like it should work, but we don't have a test specifically for this. I'll write this down as a test case, and then @sweexordious and I can plan around it today
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice! all the documentation in the proto files was a great idea/addition.
👍 👍 LGTM
@evan-forbes Can I merge then? |
might want to wait on @adlerjohn approval just to double check, but all my nits were covered |
* adds overview and valset init ADRs * cleanup * adds Data commitment ADR init * improves the valset adr * cosmetics * adds more details and removes unnecessary stuff * cosmetics * proto format + ADR updates * updates proto files + ADR * adds endblocker doc to adr * typo
…reum/Orchestrator addresses to cosmos-sdk.staking.Validator (#276) * initial orchestrator client * format proto and add necessary queries * add orchestrator logic * and encoding of data commitments and validator set changes * add orchestrator command * add relayer * add the orchestrator command * proto formatting * use correct name Co-authored-by: CHAMI Rachid <chamirachid1@gmail.com> * use correct path to endpoint Co-authored-by: CHAMI Rachid <chamirachid1@gmail.com> * get rid of GravityNonces struct * use correct name Co-authored-by: CHAMI Rachid <chamirachid1@gmail.com> * Adds QGB related ADRs (#250) * adds overview and valset init ADRs * cleanup * adds Data commitment ADR init * improves the valset adr * cosmetics * adds more details and removes unnecessary stuff * cosmetics * proto format + ADR updates * updates proto files + ADR * adds endblocker doc to adr * typo * regenerate proto * go mod tidy * use correct name Co-authored-by: CHAMI Rachid <chamirachid1@gmail.com> * handle error * begin refactor * adds data commitments and valset orchestrator processors * add the app client that queries for validator sets and data commitments * add two new methods * add query methods to appClient * WIP: update orchestrator and relayer to use the appClient * WIP: update orchestrator and relayer to use the appClient * WIP: update orchestrator and relayer to use the appClient * adds evm_client implementation * add the nil clients for the evm and mockappclient * add the new methods to the mock app client * flesh out the mock app client * flesh out app orchestrator test * adds QueryLatestValset to appClient * wip: adds orchestrator test * adds orchestrator valset test * moves orchestrator test utils to a separate file * adds orchestrator valset processEvents error check in test * rename ValidatorSignature field in DataCommitmentConfirm to ValidatorAddress * fix valset orchestrator signature test * fix last valset request in orchestrator * fix last valset request in orchestrator common test * remove unnecessary nonce increment for data commitment signing in orchestrator * fix orchestrator data commitment test * closes the mocked app client in orchestrator test * add the mock evm client * fix the command to orchestrate valsets properly * get rid of zerologger * use tmlog instead of zerolog * remove extra err handling * add comment to question nonce usage in evm client submit dataroot * adds data commitment orchestrator test * adds deploy command * adds evmRpc flag to qgb config * deubgging the qgb install * add set orchestrator command * removes set Ethereum/Orch address + fixes tests + general cleanup (#296) * removing setting the address separatly and relying on the eth/orch addresses to be defined in sdk.staking.Validator * add tx command for qgb module * added proto generated files * fixes tests * adds todo * todo * pass the keyring backend and path to app client * adds error check the newKeyedTransactorWithChainId * rename QueryLastValset1 to QueryLastValsetRequests * rename evmChainIDFlag typo * rename QueryLastValsets * adds todo * update deploy command to use the right latest valset * adds todo * updated go.mod and go.sum * update the single-node.sh script to initialize three accounts and create one validator with an orchestrator and ethereum address * imports * adds events testing main.go * remove unused variable * use sweexordious fork for cosmos sdk in the app * update go.sum * update .gitignore * update lint.yml * linter fixes * new line * linter * todo * linter checks * linter checks * linter checks * adds mutexes for race conditions Co-authored-by: CHAMI Rachid <chamirachid1@gmail.com>
* adds overview and valset init ADRs * cleanup * adds Data commitment ADR init * improves the valset adr * cosmetics * adds more details and removes unnecessary stuff * cosmetics * proto format + ADR updates * updates proto files + ADR * adds endblocker doc to adr * typo
…reum/Orchestrator addresses to cosmos-sdk.staking.Validator (celestiaorg#276) * initial orchestrator client * format proto and add necessary queries * add orchestrator logic * and encoding of data commitments and validator set changes * add orchestrator command * add relayer * add the orchestrator command * proto formatting * use correct name Co-authored-by: CHAMI Rachid <chamirachid1@gmail.com> * use correct path to endpoint Co-authored-by: CHAMI Rachid <chamirachid1@gmail.com> * get rid of GravityNonces struct * use correct name Co-authored-by: CHAMI Rachid <chamirachid1@gmail.com> * Adds QGB related ADRs (celestiaorg#250) * adds overview and valset init ADRs * cleanup * adds Data commitment ADR init * improves the valset adr * cosmetics * adds more details and removes unnecessary stuff * cosmetics * proto format + ADR updates * updates proto files + ADR * adds endblocker doc to adr * typo * regenerate proto * go mod tidy * use correct name Co-authored-by: CHAMI Rachid <chamirachid1@gmail.com> * handle error * begin refactor * adds data commitments and valset orchestrator processors * add the app client that queries for validator sets and data commitments * add two new methods * add query methods to appClient * WIP: update orchestrator and relayer to use the appClient * WIP: update orchestrator and relayer to use the appClient * WIP: update orchestrator and relayer to use the appClient * adds evm_client implementation * add the nil clients for the evm and mockappclient * add the new methods to the mock app client * flesh out the mock app client * flesh out app orchestrator test * adds QueryLatestValset to appClient * wip: adds orchestrator test * adds orchestrator valset test * moves orchestrator test utils to a separate file * adds orchestrator valset processEvents error check in test * rename ValidatorSignature field in DataCommitmentConfirm to ValidatorAddress * fix valset orchestrator signature test * fix last valset request in orchestrator * fix last valset request in orchestrator common test * remove unnecessary nonce increment for data commitment signing in orchestrator * fix orchestrator data commitment test * closes the mocked app client in orchestrator test * add the mock evm client * fix the command to orchestrate valsets properly * get rid of zerologger * use tmlog instead of zerolog * remove extra err handling * add comment to question nonce usage in evm client submit dataroot * adds data commitment orchestrator test * adds deploy command * adds evmRpc flag to qgb config * deubgging the qgb install * add set orchestrator command * removes set Ethereum/Orch address + fixes tests + general cleanup (celestiaorg#296) * removing setting the address separatly and relying on the eth/orch addresses to be defined in sdk.staking.Validator * add tx command for qgb module * added proto generated files * fixes tests * adds todo * todo * pass the keyring backend and path to app client * adds error check the newKeyedTransactorWithChainId * rename QueryLastValset1 to QueryLastValsetRequests * rename evmChainIDFlag typo * rename QueryLastValsets * adds todo * update deploy command to use the right latest valset * adds todo * updated go.mod and go.sum * update the single-node.sh script to initialize three accounts and create one validator with an orchestrator and ethereum address * imports * adds events testing main.go * remove unused variable * use sweexordious fork for cosmos sdk in the app * update go.sum * update .gitignore * update lint.yml * linter fixes * new line * linter * todo * linter checks * linter checks * linter checks * adds mutexes for race conditions Co-authored-by: CHAMI Rachid <chamirachid1@gmail.com>
Closes: celestiaorg/orchestrator-relayer#2