Skip to content
This repository has been archived by the owner on May 22, 2023. It is now read-only.

Initial implementation of app-specific actions #580

Merged
merged 40 commits into from
Oct 23, 2020
Merged

Conversation

lukasz-zimnoch
Copy link
Member

@lukasz-zimnoch lukasz-zimnoch commented Oct 14, 2020

Refs: #574
Depends on: keep-network/tbtc#739

Here we introduce the backbone which allows adding application-specific actions straight into the keep-ecdsa client. As its first use, we also add the TBTC extension which allows invoking retrieveSignerPubkey action on new deposits which are close to exceeding the signing group formation timeout. Here is a brief summary of major changes:

  • Added a new [Extensions.TBTC] section in the config which allows configuring the TBTC extension
  • Implemented a TBTC wrapper around the existing Ethereum chain implementation which takes advantage of Add Go contract bindings tbtc#739 and use these bindings to interact with TBTC contracts
  • Developed a structure for app-specific extensions allowing to handle multiple applications and chains
  • Implemented a TBTC extension with one retrieveSignerPubkey action allowing to add new actions as well
  • Created a TBTC wrapper around the local chain implementation and developed several unit test scenarios which leverage that chain

Added a new sections to the config file. It
allows to configure some properties needed
by the app-specific actions feature.
Implemented a starting point for TBTC-specific
actions. This also includes several chain
extensions needed by TBTC actions.
`Connect` function should return struct
instead of the chain interface. This is because
the caller invokes that function explicitly to
obtain a specific implementation of the chain.
That change also follows the Go
`accept interfaces return structs` principle.
Added initialization of the app-specific actions
directly in the start command.
@lukasz-zimnoch lukasz-zimnoch changed the title App specific actions Initial implementation of app-specific actions Oct 14, 2020
configs/config.toml.SAMPLE Outdated Show resolved Hide resolved
cmd/start.go Outdated Show resolved Hide resolved
pkg/chain/ethereum/tbtc_extensions.go Outdated Show resolved Hide resolved
pkg/actions/tbtc_actions.go Outdated Show resolved Hide resolved
Removed the `DepositLog` binding usage in favor
of `TBTCSystem` binding as it is the actual
contract emitting deposit lifecycle events.
Apart that, a refactoring of the config
structure has been made.
Move the app-specific parts of the chain
implementation into the application package.
Implemented the backbone for TBTC actions and
added the `retrievePubkey` action.
cmd/start.go Outdated Show resolved Hide resolved
configs/config.toml.SAMPLE Outdated Show resolved Hide resolved
cmd/start.go Outdated Show resolved Hide resolved
pkg/applications/tbtc/chain/ethereum/ethereum.go Outdated Show resolved Hide resolved
pkg/applications/tbtc/tbtc.go Outdated Show resolved Hide resolved
pkg/applications/tbtc/tbtc.go Outdated Show resolved Hide resolved
pkg/applications/tbtc/tbtc.go Outdated Show resolved Hide resolved
configs/config.toml.SAMPLE Outdated Show resolved Hide resolved
configs/config.toml.SAMPLE Outdated Show resolved Hide resolved
pkg/extensions/tbtc/chain.go Outdated Show resolved Hide resolved
pkg/chain/ethereum/tbtc.go Outdated Show resolved Hide resolved
cmd/start.go Show resolved Hide resolved
Implemented a basic unit test checking the
happy path of the retrieve pubkey TBTC extension
using a local chain implementation.
Copy link
Member

@pdyraga pdyraga left a comment

Choose a reason for hiding this comment

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

Some comments on general naming and logs.

pkg/extensions/tbtc/tbtc.go Outdated Show resolved Hide resolved
pkg/extensions/tbtc/tbtc.go Outdated Show resolved Hide resolved
pkg/extensions/tbtc/tbtc.go Outdated Show resolved Hide resolved
pkg/extensions/tbtc/tbtc.go Outdated Show resolved Hide resolved
pkg/extensions/tbtc/tbtc.go Outdated Show resolved Hide resolved
pkg/extensions/tbtc/tbtc.go Outdated Show resolved Hide resolved
pkg/extensions/tbtc/tbtc.go Outdated Show resolved Hide resolved
pkg/extensions/tbtc/tbtc.go Outdated Show resolved Hide resolved
pkg/extensions/tbtc/tbtc.go Outdated Show resolved Hide resolved
pkg/extensions/tbtc/tbtc.go Outdated Show resolved Hide resolved
@lukasz-zimnoch
Copy link
Member Author

@pdyraga all comments addressed! I'm starting tests on my local environment but I don't expect any big changes in the code. Also, I think those TODOs listed in pkg/extensions/tbtc/tbtc.go should be addressed in separate PRs.

@lukasz-zimnoch lukasz-zimnoch marked this pull request as ready for review October 20, 2020 12:04
Copy link
Member

@pdyraga pdyraga left a comment

Choose a reason for hiding this comment

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

Some additional comments, mostly about naming, docs, and logs.

Reviewing tests now.

configs/config.toml.SAMPLE Outdated Show resolved Hide resolved
go.sum Show resolved Hide resolved
internal/config/config.go Outdated Show resolved Hide resolved
pkg/extensions/tbtc/tbtc.go Outdated Show resolved Hide resolved
pkg/extensions/tbtc/tbtc.go Outdated Show resolved Hide resolved
pkg/extensions/tbtc/tbtc.go Show resolved Hide resolved
pkg/extensions/tbtc/tbtc.go Outdated Show resolved Hide resolved
pkg/extensions/tbtc/tbtc.go Outdated Show resolved Hide resolved
pkg/extensions/tbtc/tbtc.go Outdated Show resolved Hide resolved
pkg/extensions/tbtc/tbtc.go Show resolved Hide resolved
pkg/chain/local/bonded_ecdsa_keep_factory.go Outdated Show resolved Hide resolved
pkg/chain/local/local.go Outdated Show resolved Hide resolved
pkg/extensions/tbtc/tbtc_test.go Show resolved Hide resolved
pkg/extensions/tbtc/tbtc_test.go Show resolved Hide resolved
@pdyraga pdyraga added this to the v1.5.0 milestone Oct 23, 2020
Copy link
Member

@pdyraga pdyraga left a comment

Choose a reason for hiding this comment

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

🚀

@pdyraga pdyraga merged commit f49e8c4 into master Oct 23, 2020
@pdyraga pdyraga deleted the app-specific-actions branch October 23, 2020 08:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants