refactor: simplify transfer-eth-erc20/to-near #7
Conversation
chadoh
force-pushed
the
simplify-transfer-to-near
branch
from
faca9bc to
6a7a6b4
Compare
|
Looks it doesn't pass ci: |
|
@ailisp the signature of How can I update both libraries at the same time? |
|
I see, yeah this is kind of tricky, this is a workflow i used: Basically you need to create two PRs, one here one rainbow-bridge-cli, this one can merge with a failed CI, and bump/republish rainbow-bridge-lib. Then in PR in rainbow-bridge-cli use new version of rainbow-bridge-lib and CI there must pass |
| const blockData = await web3.eth.getBlock(block.number) | ||
| // is this necessary? how different are these? | ||
| // * robustWeb3.getBlock (the passed-in block) | ||
| // * robustWeb3.web3.eth.getBlock |
There was a problem hiding this comment.
@ailisp do you know the answer to this question?
There was a problem hiding this comment.
web3.eth.getBlock sometimes failed and crashed because websocket disconnection. robustWeb3 is a wrapper to web3, handle websocket disconnection, reconnect and retry. We should always use robustWeb3 (some usage of web3 hasn't been replaced by robustWeb3, but that's the plan)
| // Wait until client accepts this block number. | ||
| while (true) { | ||
| // @ts-ignore | ||
| const last_block_number = ( | ||
| await ethOnNearClientContract.last_block_number() | ||
| ).toNumber() | ||
| // Why should the client decide what's safe? | ||
| // Shouldn't MintableFungibleToken enforce this? | ||
| // And a frontend set expectations accordingly? |
There was a problem hiding this comment.
@ailisp and these questions? To me it seems like we should:
- set
25in MintableFungibleToken - adjust expectations accordingly here
- never call
ethOnNearClientContract.block_hash_safe
There was a problem hiding this comment.
MintableFungibleToken does enforce this, so if frontend doesn't wait block_hash_safe and continue to next step(mint), it will be rejected by MintableFungibleToken. So we also wait in frontend
There was a problem hiding this comment.
25 is mentioned in the blog post introducing Bridge (https://near.org/blog/eth-near-rainbow-bridge/). I would expect it's part of MintableFungibleToken. I did not expect it to be part of EthOnNearClient, but the existence of ethOnNearClientContract.block_hash_safe indicates otherwise.
Sounds like I could get away with never calling block_hash_safe on the frontend, if the frontend makes the correct assumptions.
There was a problem hiding this comment.
However, here in rainbow-bridge-lib, I think it's worth checking both
There was a problem hiding this comment.
Sounds like I could get away with never calling block_hash_safe on the frontend, if the frontend makes the correct assumptions.
Yes if contract is configured so. Need to check with @nearmax if it's really 25 in our deployment, it's probably a bigger number.
`proof_locker` and `proof` were calculated and saved during discrete steps of the transfer process, rather than as-needed when minting. This made it hard to understand where the data was coming from and why it was being calculated while reading through the code. Additionally, there was a fair bit of indirection and awkward code throughout. This has been cleaned up.
chadoh
force-pushed
the
simplify-transfer-to-near
branch
from
6a7a6b4 to
886a928
Compare
proof_lockerandproofwere calculated and saved during discrete steps of the transfer process, rather than as-needed when minting. This made it hard to understand where the data was coming from and why it was being calculated while reading through the code.Additionally, there was a fair bit of indirection and awkward code throughout. This has been cleaned up.