Outstanding Questions

[okwme]

After running through a successful deployment of Peggy as part of the Eth Denver burner chain I'm left with the following question and comments:

• In Oracle.sol within the newOracleClaim() function, why is the signature from the validator necessary? The message itself is signed by the same address already.
• In Oracle.sol within the checkBridgeProphecy() function, why does it need to have the onlyOperator() modifier when it is in fact a public view function?
• In BridgeBank.sol why is there only a lock and not a burn. Shouldn't SDK native assets which are present on the EVM chain be burned when moved back to the SDK chain instead of just locked?
• It seems the token denomination for unlocking on the SDK chain is taken from the token symbol. This can come from any arbitrary token that the BridgeBank has permission to mint on and has been added to the whitelist. I would imagine a peggy bridge that allowed any asset to be safely transferred would be preferrable to one which only allowed whitelisting tokens which have no name collisions with assets already on the chain. Furthermore what happens if the two tokens are separate but just so happen to be represented with the same denom on both sides? It feels like the wrapped denominations should have some sort of prefix denoting their origin.
• There's still the outstanding issues of events which were not heard by the relayer and checking to make sure any previously missed events can still be relayed as discussed in issue How to relay a missed transaction? #124
• And outstanding issues regarding EVM native assets which were locked instead of burned when transferring back to the EVM chain in issue How to resolve incorrect lock? #123

[denalimarsh]

That's awesome, glad to hear that Peggy was successfully deployed at Eth Denver! These are great questions, thanks for digging into the codebase @okwme

• The signature is used to validate that the validator signed a message containing the prophecy's information, but since the message isn't compared to the prophecy's actual information the check looks to be redundant. Therefore the message and signature could be removed entirely, with the address alone denoting that this validator agrees with the prophecy claim information in event LogNewProphecyClaim. However, retaining the message/signature and parameterizing the validator's address would allow for the aggregation and submission of oracle claims by another party (such as the relayer or the intended recipient).
• The original reason for the onlyOperator() modifier is that the checkBridgeProphecy() function was intended to be used by the operator to check the status of bridge prophecies at a reduced gas cost such that checking prophecy status inside a cron job before attempting to complete them is cheaper than paying for any failed completion attempts. It can be removed.
• The technical spec for this MVP includes lock/unlock of ERC20 tokens, allowing for bidirectional cross-chain asset transfers. Holding the locked tokens unless they're returned from Cosmos allows for secure transfers, but I agree that future work could be done to allow for the burning of SDK native assets on Ethereum. The feature could introduce several potential issues and should first be properly specced out.
• Please see Handle TokenContractAddress in Peggy and deal with duplicate symbols across token addresses #69, a discussion regarding some of the pitfalls of cross-chain token identification. ChainID and TokenContractAddress were introduced to help mitigate these concerns. Adding an origin-denoting prefix seems like a reasonable idea, perhaps we can leverage the registered coin indexes from SLIP-0044. But without the token whitelist a malicious user could still submit locks of dummy ERC20 tokens with the same denom/id and receive valid tokens on Cosmos.
• We'll address issues How to resolve incorrect lock? #123, How to relay a missed transaction? #124 shortly.

Hopefully, this gives some insight into prior development - @musnit may be able to weigh in here and shed further light on the project's scope and some of our technical decisions.

[musnit]

Agree with all your responses, thanks!

[okwme]

Awesome, thanks for the clarifications @denalimarsh and @musnit . From this I gather there are the following actionables:

• Create a new issue to discuss the best plan for supporting signed messages or removing them.
  • Utilize or remove signed messages within validator claims #148
• Remove OnlyOperator() from checkbridgeProphecy() (I'm still not clear on how this modifier reduced gas costs tho. Does the modifier reduce gas for the operator? shouldn't it increase it since there are more logic steps to take? And couldn't it still be used within a cron job to check the prophecy without being restricted by the modifier?)
• Create a new issue to make a spec and discuss burning cosmos based tokens after they leave the eth chain. I'm still not sure why it was deemed necessary/ok to burn the eth assets on the cosmos chain but not the cosmos assets on the eth chain? They're essentially performing the same actions just inverted right?
  • Burn Cosmos Native Eth Tokens After Use #147
• Move further discussion about token denoms to the original Handle TokenContractAddress in Peggy and deal with duplicate symbols across token addresses #69