Conversation
bonnie57
requested review from
AndyBoWu,
DonFungible,
LeoHChen,
allenchuang,
edisonz0718 and
jacob-tucker
as code owners
SoYoung210
left a comment
SoYoung210
left a comment
There was a problem hiding this comment.
for v1.3, do we perhaps need a @deprecated annotation for mintAndRegisterIpAndMakeDerivativeAndDistributeRoyaltyTokens?
https://github.com/storyprotocol/protocol-periphery-v1/pull/147/files#
Hi @SoYoung210, I confirmed this with @allenchuang. We will implement the newest code, which will not include deprecated methods. So, the next PR will import the latest methods and remove deprecated methods, and this PR will not be released in a new version. The pr is to make sure compatible v1.3 implementation. |
| } else { | ||
| const txHash = | ||
| await this.groupingWorkflowsClient.mintAndRegisterIpAndAttachLicenseAndAddToGroup(object); | ||
| await this.groupingWorkflowsClient.mintAndRegisterIpAndAttachLicenseAndAddToGroup2( |
There was a problem hiding this comment.
why was mintAndRegisterIpAndAttachLicenseAndAddToGroup2 created instead of mintAndRegisterIpAndAttachLicenseAndAddToGroup being updated? Do we still need the old function? I noticed a few functions are like this.
nvm I got it, looks like there are are multiple solidity functions with that name because the original one is deprecated. I guess no good ways around that until the deprecated ones are removed?
There was a problem hiding this comment.
Yes, we are the same page.
| }, | ||
| }, | ||
| // { | ||
| // name: "MockERC20", |
There was a problem hiding this comment.
are we planning to bring this back? we should just remove the code unless we have a reason to keep it.
There was a problem hiding this comment.
yes, we need it.
There was a problem hiding this comment.
can you add a comment here on why its commented out then? maybe a todo or something so its clear to other devs.
|
|
||
| const func = functions[overloadIndex || 0]; | ||
|
|
||
| const getTypeString = ( |
There was a problem hiding this comment.
are we able to use viem utils to get function sig? seems viem's getFunctionSignature and getAbiItem might be useful?
There was a problem hiding this comment.
Yeah, But I think it doesn't meet our requirements. The function can reduce our workload, as we don't need to manually add ABI.
bonnie57
changed the title
3 participants
Description
maxMintFee,maxRts, andmaxRevenueSharefor the related methodssnapshotandworkflowmethodsNext step:
Test
Related Issue
Implement Protocol v1.3 #370
Attention
Once v1.3 is ready, merge the code into the
v1.3branch. Then, merge it into thedevbranch so that it will not block other development in SDK.