omni / poa-bridge

POA <-> Ethereum bridge for self transfers of POA native token to POA20 (ERC20 representation). Not supported. Use TokenBridge instead
https://github.com/poanetwork/token-bridge
GNU General Public License v3.0
80 stars 38 forks source link

Extend tracebility of deposits transactions #9

Closed akolotov closed 6 years ago

akolotov commented 6 years ago

Currently every bridge instance sends deposit() to the Foreign side to confirm transfer of funds. But only transaction which allow to reach requiredSignatures limit generates an event.

<a href="https://www.websequencediagrams.com/cgi-bin/cdraw?lz=dGl0bGUgYnJpZGdlIGRlcG9zaXQgKGZsb3cgIHdpcmggMwAWB3MgaW4gYWN0aW9uKQoKcGFydGljaXBhbnQgIkhvbWUgQWNjb3VudCIgYXMgSEEADhNzaWRlXG5Db250cmFjACEHQwA7DkIAgQIFXG5JbnN0YW5jZSAjMQBLBUIxAAggMgAiBjIAMCAzAEoGMwCBMw5Gb3JlaWduAIEPFEZDCgpIQS0-K0hDOiBldGhlcgpub3RlIGxlZnQgb2YgSEMKICAgIGFzc29zaWF0ZWQACgV3aXRoIHR4IGlkCmVuZCBub3RlCkhDLT5CMTogRACCXwYoc2VuZGVyLCB2YWx1ZSkAGQYyAAIdLUIzACYZAIEQBW92ZXIgQjEsIEIyLCBCMwCBEgh1bWVkIHRoYXQAgSYFAHIGID09IHJlY2lwaWVudACBGwpCMS0-K0ZDOgCEAwgoABsJAIEiBywAgVEGKQpGQy0-LUZDOiAxc3Qgc2lnbmF0dXJlIHJlY2VpdmVkCkIyABkxMm5kADUVMwBoLEZDOiAzcgA0FQCBKgkic3RhdGVcbmNvbW1pdGVkIgo&s=rose" target="_blank"> <img src="https://www.websequencediagrams.com/cgi-bin/cdraw?lz=dGl0bGUgYnJpZGdlIGRlcG9zaXQgKGZsb3cgIHdpcmggMwAWB3MgaW4gYWN0aW9uKQoKcGFydGljaXBhbnQgIkhvbWUgQWNjb3VudCIgYXMgSEEADhNzaWRlXG5Db250cmFjACEHQwA7DkIAgQIFXG5JbnN0YW5jZSAjMQBLBUIxAAggMgAiBjIAMCAzAEoGMwCBMw5Gb3JlaWduAIEPFEZDCgpIQS0-K0hDOiBldGhlcgpub3RlIGxlZnQgb2YgSEMKICAgIGFzc29zaWF0ZWQACgV3aXRoIHR4IGlkCmVuZCBub3RlCkhDLT5CMTogRACCXwYoc2VuZGVyLCB2YWx1ZSkAGQYyAAIdLUIzACYZAIEQBW92ZXIgQjEsIEIyLCBCMwCBEgh1bWVkIHRoYXQAgSYFAHIGID09IHJlY2lwaWVudACBGwpCMS0-K0ZDOgCEAwgoABsJAIEiBywAgVEGKQpGQy0-LUZDOiAxc3Qgc2lnbmF0dXJlIHJlY2VpdmVkCkIyABkxMm5kADUVMwBoLEZDOiAzcgA0FQCBKgkic3RhdGVcbmNvbW1pdGVkIgo&s=rose" alt="bridge deposit (simple flow)" width="750" />

Proposal for changes


In order to simplify issues investigation process it makes sense to generate events on every unique and valid depost() transactions.

The suggestion is to raise SignedForDeposit(address indexed, bytes32) with the bridge address and id of the transaction originating deposit request on the Home (left) side:

Prior to merge of these changes it is necessary to understand how gas consumption is being increased and get agreement that it is acceptable.

rstormsf commented 6 years ago

I support this idea. Can you specify which method should fire this event? @akolotov I assume deposit() on ForeignBridge and same for transferAndCall

akolotov commented 6 years ago