-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Connectors v1 #16
Connectors v1 #16
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, but some of the comments in the tests are misleading or inaccurate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really thorough testing, nice :)
…ction for testing.
Use a cent-chain generated message to test AddTranche encoding and decoding functions.
* Use cent-chain generated UpdateMember for decoding * Drop toBytes16
* wip: add transfer test * move memberlist check from connectors to restricted token * fix tests * revert restricted.sol changes * revert unwanted changes to erc20 and memberlist * fix test
* remove domainLookup * require destination domain to be cent chain * fix tests * clean up * fix up * rename Domains to Domain * Add new domain lookup * update domain enum * move formatDomain to messages.sol * fix tests * update enum and domain encoding * update domain encoding to output bytes9 * Update src/Connector.sol * update domain encoding and add more tests --------- Co-authored-by: Jeroen Offerijns <[email protected]>
* Fix transfer encoding / decoding We need to encode address as 32 bytes and take in consideration when decoding. Also, amount should be encoded as a u128. * Fix transfer decoding * Cover transfer to centrifuge * Fix docs - thanks @AStox
uint64 poolId, | ||
bytes16 trancheId, | ||
address user, | ||
uint256 amount | ||
) public onlyRouter { | ||
RestrictedTokenLike token = RestrictedTokenLike(tranches[poolId][trancheId].token); | ||
require(address(token) != address(0), "CentrifugeConnector/unknown-token"); | ||
require(token.hasMember(user), "CentrifugeConnector/not-a-member"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bumping this ^ @NunoAlexandre @AStox
uint64 poolId, | ||
bytes16 trancheId, | ||
address user, | ||
uint256 amount | ||
) public onlyRouter { | ||
RestrictedTokenLike token = RestrictedTokenLike(tranches[poolId][trancheId].token); | ||
require(address(token) != address(0), "CentrifugeConnector/unknown-token"); | ||
require(token.hasMember(user), "CentrifugeConnector/not-a-member"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gotcha. I would keep it like this for now, to not change the existing restricted token implementation. But let's discuss this a bit more and we can still change it in V2 optionally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's happening 🌟 🚀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! 💯
To Do
Messages.t.sol
@NunoAlexandredomain
to theTransfer
message - see Connectors v1 #16 (comment) @AStox