Skip to content
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

Add superfluid connector #244

Closed
wants to merge 2 commits into from

Conversation

CoffeeFam84
Copy link

Add superfluid connector to arbitrum, avalanche, optimism and polygon.

@github-actions
Copy link

Finished status checks

@pradyuman-verma pradyuman-verma self-requested a review July 13, 2022 21:33
* @param userData The user provided data
*
*/
event FlowUpdated(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Every event should start with Log...

Suggested change
event FlowUpdated(
event LogFlowUpdated(

*
*/
event FlowUpdated(
ISuperfluidToken indexed token,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we emit it as address?

Comment on lines 12 to 16
ISuperfluid host = ISuperfluid(0xCf8Acb4eF033efF16E8080aed4c7D5B9285D2192);
IInstantDistributionAgreementV1 ida =
IInstantDistributionAgreementV1(
0x2319C7e07EB063340D2a0E36709B0D65fda75986
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be an internal constant.

Suggested change
ISuperfluid host = ISuperfluid(0xCf8Acb4eF033efF16E8080aed4c7D5B9285D2192);
IInstantDistributionAgreementV1 ida =
IInstantDistributionAgreementV1(
0x2319C7e07EB063340D2a0E36709B0D65fda75986
);
ISuperfluid internal constant host = ISuperfluid(0xCf8Acb4eF033efF16E8080aed4c7D5B9285D2192);
IInstantDistributionAgreementV1 internal constant ida =
IInstantDistributionAgreementV1(
0x2319C7e07EB063340D2a0E36709B0D65fda75986
);

);

//initialize InitData struct, and set equal to cfaV1
CFAv1Library.InitData public cfaV1 =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
CFAv1Library.InitData public cfaV1 =
CFAv1Library.InitData internal constant cfaV1 =

payable
returns (string memory _eventName, bytes memory _eventParam)
{
IERC20(superToken.getUnderlyingToken()).approve(
Copy link
Contributor

@pradyuman-verma pradyuman-verma Jul 31, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can get the amount using getId as well, and this amount should be passed to approve func

Suggested change
IERC20(superToken.getUnderlyingToken()).approve(
uint256 amt_ = getUint(getId, amount);
IERC20(superToken.getUnderlyingToken()).approve(

@pradyuman-verma
Copy link
Contributor

@CoffeeFam84

  • The use of getId and setId is missing in functions.
  • Variables should be internal constant.
  • You also need to return the _eventName, _eventParam for each function.

You can check aave v3 connector, for example - https://github.com/Instadapp/dsa-connectors/blob/main/contracts/polygon/connectors/aave/v3/main.sol

@CoffeeFam84
Copy link
Author

CoffeeFam84 commented Aug 1, 2022

@pradyuman-verma
For getId and setId, we don't need them in functions, in my point of view.
And for emitting events, every function in superfluid are emitting events already. so do we need to emit them again in instadapp?

@pradyuman-verma
Copy link
Contributor

Yes we do, these events help us do to tracking related to DSAs

@pradyuman-verma
Copy link
Contributor

@pradyuman-verma For getId and setId, we don't need them in functions, in my point of view. And for emitting events, every function in superfluid are emitting events already. so do we need to emit them again in instadapp?

@CoffeeFam84 setId and getId are used to communicate between simultaneous spells, that's why those are necessary

@github-actions
Copy link

github-actions bot commented Aug 3, 2022

Finished status checks

Comment on lines +21 to +37
CFAv1Library.InitData internal cfaV1 =
CFAv1Library.InitData(
host,
//here, we are deriving the address of the CFA using the host contract
IConstantFlowAgreementV1(
address(
host.getAgreementClass(
keccak256(
"org.superfluid-finance.agreements.ConstantFlowAgreement.v1"
)
)
)
)
);

// declare `_idaLib` of type InitData and assign it the host and ida addresses
IDAv1Library.InitData internal _idav1Lib = IDAv1Library.InitData(host, ida);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be constant as well

* @param setId ID stores the amount of token.
*/
function deleteFlow(
address sender,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need sender, the sender will be address(this)

payable
returns (string memory _eventName, bytes memory _eventParam)
{
CFAv1Library.deleteFlow(cfaV1, sender, receiver, superToken, userData);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
CFAv1Library.deleteFlow(cfaV1, sender, receiver, superToken, userData);
CFAv1Library.deleteFlow(cfaV1, address(this), receiver, superToken, userData);

*/
function createFlowByOperator(
ISuperToken superToken,
address sender,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
address sender,

{
CFAv1Library.createFlowByOperator(
cfaV1,
sender,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
sender,
addresss(this),

*/
function updateFlowByOperator(
ISuperToken superToken,
address sender,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
address sender,

{
CFAv1Library.updateFlowByOperator(
cfaV1,
sender,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
sender,
address(this),

*/
function deleteFlowByOperator(
ISuperToken superToken,
address sender,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
address sender,

{
CFAv1Library.deleteFlowByOperator(
cfaV1,
sender,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
sender,
address(this),

returns (string memory _eventName, bytes memory _eventParam)
{
IDAv1Library.distribute(_idav1Lib, superToken, id, amount, userData);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can use getId here to exact amount.

@thrilok209
Copy link
Member

Since no activity from the past 1 month from the contributor, closing this pr for now.

Later it can be re-open suggested changes are made.

Thank you

@thrilok209 thrilok209 closed this Sep 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants