Chain Adapter Implementation for BMR

[MuhammedIrfan]

Overview

The existing implementation of BMR is coupled with specific chain logics which need to be rewritten based on the chain's specifications. Due to this it creates an overhead for the developers to accommodate new chains and increases cost and risk factors as they need to test the complete BMR implementation.

Implementing adapters for chains realizes the goal of ICON to hyper-connect the world by enabling the community to just focus on creating and reusing chain adapters for BMR.

Existing Implementation

As per the existing implementation the code is coupled with chain specifics as in reference [chain.go#L560, chain.go#L573, chain.go#L499, chain.go#L137 etc..] which creates an overhead for implementing BMR for other chains.

Suggestion

We recommend creating an interface for the high level BMR implementation and adapters for specific chains to adapt to these implementations without affecting the core logic of BMR.

• Interfaces Required
• Block monitoring interface
• Proof building interface
• Message sending interface

Impact on other teams

Implementing adapters can ease the development of BMR for multiple chains and provide a standard interface. As we have working groups working to create interoperability with multiple chains we need to bring in a standard where the adapters should follow certain specification, At times the team might require to create adapters with custom apis for specific chains which might not be available in go.

[jspark-icon]

'Sender' and 'Receiver' interfaces are designed as you suggested (

btp/cmd/btpsimple/module/type.go

Line 88 in f28c29b

type Sender interface {

)

'Receiver' has responsibility which is collecting proofs from source chain and generate. 'type ReceiveCallback func(bu *BlockUpdate, rps []*ReceiptProof)' shows that.
'Sender' doesn't need to know how to make the proofs, only focus send transaction to destination chain as segmentation.

[MuhammedIrfan]

True @jspark-icon . In a way it is decoupled. But we would like to have abstraction in much higher level so that

• Users can implement adapters for new chain and integrate that without affecting underlying bmr implementation.
• We can breakdown the architecture to multiple services so that new chain can be added on the go without compiling bmr

[trmaphi]

The current BMR implementation distinguishes Chain bases on BTP address. We already have a place for the adatper in the implementation of SimpleChain Constructor

func NewSimpleChain(cfg *Config, w wallet.Wallet, l log.Logger) (*SimpleChain, error) {
	s := &SimpleChain{
		src: cfg.Src.Address,
		dst: cfg.Dst.Address,
		w:   w,
		l: l.WithFields(log.Fields{log.FieldKeyChain:
		//fmt.Sprintf("%s->%s", cfg.Src.Address.NetworkAddress(), cfg.Dst.Address.NetworkAddress())}),
		fmt.Sprintf("%s", cfg.Dst.Address.NetworkID())}),
		cfg: cfg,
		rms: make([]*module.RelayMessage, 0),
	}
	s._rm()

        // These lines can use to build adapter base on BTP address
	s.r = icon.NewReceiver(cfg.Src.Address, cfg.Dst.Address, cfg.Src.Endpoint, cfg.Src.Options, s.l)
	s.s = icon.NewSender(cfg.Src.Address, cfg.Dst.Address, w, cfg.Dst.Endpoint, cfg.Dst.Options, s.l)

	if err := s.prepareDatabase(cfg.Offset); err != nil {
		return nil, err
	}
	return s, nil
}

The current ICON adapter is located icon module. For example, we have another implementation of let's say Substrate-based chain, we can place them in cmd/btpsimple/module/substate.

tree cmd/btpsimple
cmd/btpsimple
├── chain
│   ├── chain.go
│   └── config.go
├── main.go
└── module
    ├── btpaddress.go
    ├── error.go
    ├── icon
    │   ├── client.go
    │   ├── receiver.go
    │   ├── sender.go
    │   ├── serialize.go
    │   └── type.go
    ├── substrate
    │   ├── receiver.go
    │   └── sender.go
    └── type.go

[jspark-icon]

Of course, factory of 'Receiver' and 'Sender' is required at chain.go#L609 with BTPAddress.BlockChain()

[trmaphi]

I started the implementation for adapters in https://github.com/icon-project/btp/blob/icondao/cmd/btpsimple/chain/chain.go#L610-L641

[trmaphi]

@jspark-icon In Sender interface design , the Sender.MonitorLoop() and Sender.StopMonitorLoop() methods are using data from source chain. Because of that, I think we should consider place them with Receiver Interface.

type Sender interface {
	Segment(rm *RelayMessage, height int64) ([]*Segment, error)
	UpdateSegment(bp *BlockProof, segment *Segment) error
	Relay(segment *Segment) (GetResultParam, error)
	GetResult(p GetResultParam) (TransactionResult, error)
	GetStatus() (*BMCLinkStatus, error)
	// Move these methods in Receiver
	MonitorLoop(height int64, cb MonitorCallback, scb func()) error
	StopMonitorLoop()
        //
	FinalizeLatency() int
}

[jspark-icon]

There is no any dependency of source chain in Sender.MonitorLoop and StopMonitorLoop methods.

[trmaphi]

I checked

btp/cmd/btpsimple/module/icon/sender.go

Lines 339 to 360 in f28c29b

	func (s *sender) MonitorLoop(height int64, cb module.MonitorCallback, scb func()) error {
	br := &BlockRequest{
	Height: NewHexInt(height),
	}
	return s.c.MonitorBlock(br,
	func(conn *websocket.Conn, v *BlockNotification) error {
	if h, err := v.Height.Value(); err != nil {
	return err
	} else {
	return cb(h)
	}
	},
	func(conn *websocket.Conn) {
	s.l.Debugf("MonitorLoop connected %s", conn.LocalAddr().String())
	if scb != nil {
	scb()
	}
	},
	func(conn *websocket.Conn, err error) {
	s.l.Debugf("onError %s err:%+v", conn.LocalAddr().String(), err)
	_ = conn.Close()
	})

.

@jspark-icon, Sender interface does use the icon.Client.MonitorBlock() as dependency in the implementation. That's why I suggest it should belong to Receiver interface.

[canhlinh]

@trmaphi in this line icon.NewSender(cfg.Src.Address, cfg.Dst.Address, w, cfg.Dst.Endpoint, cfg.Dst.Options, s.l) . The endpoint is cfg.Dst.Endpoint. So Sender monitors the destination chain not the source chain.

[mohamedelshami]

It's my understanding that the Sender monitors the source (the source is where the transaction is sent from) but I think the confusion is when the Sender writes updates to the dst database the file is named after the Dst database.

database, err := db.Open(s.cfg.AbsBaseDir(), string(DefaultDBType), s.cfg.Dst.Address.NetworkAddress())

[trmaphi]

I'm having some problems with the wallet implementation of the Ethereum client. I suggest we put all of the wallet implementations in chain-specific modules?

[mohamedelshami]

We're also incorporating Etheruem-go client wallet, it does make sense to move Wallet code to client-specific, however the keystore configuration is standard.

[cow-hs]

I hope to implement a wallet corresponding to "common/wallet/<walletType>"
You also need to implement a Factory that creates a newSimpleChain instance that selects the wallet used by main.go

[trmaphi]

We implemented https://github.com/icon-project/btp/blob/icondao/common/wallet/wallet_evm.go

[trmaphi]

Request for change in module interfaces to adapt a new RelayMessage structure in Prachain.

Prachain RelayMessage structure compares to

btp/cmd/btpsimple/module/type.go

Lines 3 to 43 in d73d1cf

	type BlockWitness struct {
	Height int64
	Witness [][]byte
	}
	type BlockUpdate struct {
	Height int64
	BlockHash []byte
	Header []byte
	Proof []byte
	}
	type BlockProof struct {
	Header []byte
	BlockWitness *BlockWitness
	}
	type ReceiptProof struct {
	Index int
	Proof []byte
	EventProofs []*EventProof
	Events []*Event
	}
	type EventProof struct {
	Index int
	Proof []byte
	}
	type Event struct {
	Next BtpAddress
	Sequence int64
	Message []byte
	}
	type RelayMessage struct {
	From BtpAddress
	BlockUpdates []*BlockUpdate
	BlockProof *BlockProof
	ReceiptProofs []*ReceiptProof
	Seq uint64
	HeightOfDst int64
	Segments []*Segment
	}

.

I want to replace BlockUpdate, BlockProof, ReceiptProofs structs becomes to generic interface{}

[trmaphi]

As discussed, we still keep interfaces of BlockUpdate, BlockProof, ReceiptProofs as it is. The rationale behind this decision is:

• BlockUpdate.Proof can be used to receive RLP encoded of extra fields/structs
• ReceiptProof.Proof can be used to receive RLP encoded of extra fields/structs. If EventProofs or Events are empty, we just left them be, RLP doesn't care about the extra bytes, and these bytes are not big to affect transaction size.

[trmaphi]

• BlockUpdate from RelayMessage actually built from BlockUpdate.Proof
• ReceiptProof from RelayMessage actually built from ReceiptProof.Proof
  So no problem.

[MuhammedIrfan]

Hi team, We are implementing adapter for BMR. Our intention is to make BMR implementation more generic and easier to integrate new chains, As part of it we have broken down the chain implementation into two parts base and chain-specific modules.
We intend to make sender and receiver generic. and inject the client based on chains. please find below the implementations we have done.

• Introduced adapter - Change Sender and Receiver based on btp address during BMR startup. chain.go side effect-import chain.go adapter.go client.go registry.go
• Generic base module implementation: This package contains sender and receiver. base package
• Made client implementation modular
• Decoupled RPC API
• Added Observable: For Blockmonitoring receiver.go sender.go client.go client.go
• TODO: Wallets adapter implementation
• TODO: Add unit tests for client methods
• TODO: Complete remaining testing

kindly let us know any suggestion and improvements