Skip to content
This repository was archived by the owner on Mar 13, 2023. It is now read-only.

Review chain_id settings in ethereum transact, should follow EIP 155 #1273

Closed
hackfisher opened this issue Jun 10, 2022 · 4 comments · Fixed by #1318
Closed

Review chain_id settings in ethereum transact, should follow EIP 155 #1273

hackfisher opened this issue Jun 10, 2022 · 4 comments · Fixed by #1318
Assignees
@boundless-forest
Labels
C-EVM [Component] Something about EVM P-Mid [Priority] Median S-Need Audit PR contains changes to fund-managing logic that should be properly reviewed and externally audited

Comments

@hackfisher
Copy link
Contributor

darwinia-common/frame/dvm/ethereum/src/lib.rs

Line 1038 in b31765c

1 * 2 + 36,

https://eips.ethereum.org/EIPS/eip-155
@hackfisher hackfisher added P-Mid [Priority] Median S-Need Audit PR contains changes to fund-managing logic that should be properly reviewed and externally audited labels Jun 10, 2022
@boundless-forest
Copy link
Member

I have thought that after #1239 is done, the internal transaction is useless anymore. But, solidity callback #1227 rely on the internal _transact again. I didn't find a better way to avoid this so far.

@boundless-forest boundless-forest added the C-EVM [Component] Something about EVM label Jun 10, 2022
@hackfisher
Copy link
Contributor Author

I have thought that after #1239 is done, the internal transaction is useless anymore. But, solidity callback #1227 rely on the internal _transact again. I didn't find a better way to avoid this so far.

This part related internal transact could be removed, but other places require review and check too.

@boundless-forest
Copy link
Member

This part meant ? in the solidity callback?

@hackfisher
Copy link
Contributor Author

This part meant ? in the solidity callback?

Internal transaction related codes:

darwinia-common/frame/dvm/ethereum/src/lib.rs

Line 1038 in b31765c

1 * 2 + 36,

@boundless-forest boundless-forest linked a pull request Jul 4, 2022 that will close this issue
Avoid Using Hardcode ChainId for Internal Transaction #1318
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@boundless-forest boundless-forest

Labels
C-EVM [Component] Something about EVM P-Mid [Priority] Median S-Need Audit PR contains changes to fund-managing logic that should be properly reviewed and externally audited
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Avoid Using Hardcode ChainId for Internal Transaction darwinia-network/darwinia-common
2 participants
@hackfisher @boundless-forest