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

chore: fix build and wrapper generation & regenerate #8

Merged
merged 2 commits into from
Nov 16, 2021

Conversation

liamsi
Copy link
Member

@liamsi liamsi commented Nov 16, 2021

Tests will still fail but at least the build works again.

Fixes #7

@liamsi liamsi requested a review from adlerjohn as a code owner November 16, 2021 13:48
@liamsi liamsi marked this pull request as draft November 16, 2021 13:53
@liamsi liamsi removed the request for review from adlerjohn November 16, 2021 13:53
@liamsi
Copy link
Member Author

liamsi commented Nov 16, 2021

There are still some things to fix. Hence back to draft.

@liamsi liamsi force-pushed the ismail/fix-build_and_regenerate-wrappers branch from d5630dd to df82116 Compare November 16, 2021 17:21
@liamsi liamsi changed the title fix build and wrapper generation & regenerate chore: fix build and wrapper generation & regenerate Nov 16, 2021
@liamsi liamsi requested a review from adlerjohn November 16, 2021 18:11
@liamsi liamsi marked this pull request as ready for review November 16, 2021 18:12
ValsetUpdateEvent *wrappers.PeggyValsetUpdatedEvent
ERC20DeployedEvent *wrappers.PeggyERC20DeployedEvent
EventNonce uint64
ValsetUpdateEvent *wrappers.QuantumGravityBridgeValidatorSetUpdatedEvent
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The event PeggyTransactionBatchExecutedEvent was renamed in the contract to MessageTupleRootEvent. You could argue it's the only actual critical part of the contracts that needed to be modified. This PR is fine I think, but we need to replace the function that watched for PeggyTransactionBatchExecutedEvent with one that watches for MessageTupleRootEvent in the relayer (not sure why it's under the orchestrator).


// RelayBatches checks the last validator set on Ethereum, if it's lower than our latest valida
// set then we should package and submit the update as an Ethereum transaction
func (s *peggyRelayer) RelayBatches(ctx context.Context) error {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably what we want to replace with code to relay message tuple roots.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I but I feel this PR was already way, way too large 😬

There are a few things that need to be done:

  • integration tests need to be cleaned up
  • message tuple roots creating and add in that method here (above) and in the corresponding interface
  • ValeSetRootHash needs to be created (note that the validators here used do additionally sign the valsetUpdates with a different signing key from the tendermint one as far as I understand; just noting that as it might be easier to keep it that way instead of changing everything to ecdsa in tendermint directly)
  • ...

Copy link
Member

@adlerjohn adlerjohn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lots of commented-out code, but that's fine. Should have enough info just from that to know where modifications need to be done.

@liamsi liamsi merged commit d9ccfe7 into master Nov 16, 2021
@liamsi liamsi deleted the ismail/fix-build_and_regenerate-wrappers branch November 16, 2021 21:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clean up 🧹 Clean up contracts
2 participants