-
Notifications
You must be signed in to change notification settings - Fork 11
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
feat: add deploy contract command #242
Conversation
23b6c8a
to
57f6cdf
Compare
paramTypes.string, | ||
true, | ||
) | ||
.setAction(async ({ name, chain, fee, env, migrate, ...args }: DeployArgs, { config, run }) => { |
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.
How about extracting the code of deployment so that we can use it to deploy the infra scripts?
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.
How is it going |
The contract module of kuai needs a way to manage and organize, and some parts of the current contract management by For example: So For this reason, the following three structures are defined, where the export interface ContractManager {
contracts: KuaiContract[]
newContract: (name: string, template?: 'rust' | 'c' | 'c-sharedlib')
build: (name: string) => void
createDeployTx: (name: string, deployer: Address) => Promise<Transaction>
createUpgradeTx: (name: string, deployer: Address) => Promise<Transaction>
commitTx: (name: string, witnesses: string[]) => Promise<Hash>
}
interface KuaiContract {
name: string
templateType: string
srcPath: string
binPath: string
deployType: 'data' | 'typeId'
migrations: KuaiContractMigration[]
}
interface KuaiContractMigration {
chain: string
txHash: Hash
index: number
dataHash: Hash
typeId: Hash
} Specific implementation details: deploy process: |
How about accepting a Ref: #233 |
I think we can support signed by |
Upvote, only |
Does support for signature by "kuai" mean wrapping "ckb-cli" or using the keystore directly? |
It's about the detail of implementation, but |
How is it going @PainterPuppets |
Have implemented the ContractManager part, is doing the deployment of the signature part, because the keystore built into the kuai spend more time, so are packing ckb-cli signature part, is expected to be completed and submit tomorrow |
Looking forward to it |
How is it going |
57f6cdf
to
9835de2
Compare
4b0219b
to
1d45bd4
Compare
Please update the usage in the PR message and I'll request some reviews from outside teams that may use |
Updated, only the ckb-cli signature method is supported for now, but the extension point is left, I don't know what the outside teams is using for the signature, I can go add it |
Should kuai's user keep their private key on the disk to sign the transaction with If so, signing a transaction with lumos should be considered, and the private key should be decrypted from a keystore file instead of reading a private key file |
But it could be treated as a low-priority one, signing a transaction with |
No, if sign with |
I see, so |
I thought ahead about the multisg implementation, the specific logic part does not conflict with multisig, at the cli level it will probably look like this
|
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.
The tx information in block 0 is stable, can we use const variables to define this information?
import { RPC, config, utils } from '@ckb-lumos/lumos' | ||
|
||
// Special cells in genesis transactions: (transaction-index, output-index) | ||
const SpecialCellLocation: { |
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.
Better to add a reference of this locations
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.
I'm not quite sure where this reference can be found, these magic location is what I see in the source code of ckb-cli
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.
Please refer to the code of ckb-cil, and I'll open an issue in ckb-cli repo for detailed info
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.
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.
My bad, I didn't notice the comment above, which clearly implies the definition of [number, number]
is [transaction-index, output-index]
Among them, SIGNHASH_GROUP
, MULTISIG_GROUP
, DAO
can be found at https://github.com/nervosnetwork/rfcs/blob/master/rfcs/0024-ckb-genesis-script-list/0024-ckb-genesis-script-list.md
SIGHASH
, MULTISIG
are not used normally. I'm not sure if these two will be used in our project.
Besides, we can use labeled tuple for readability
...generateGenesisScriptConfig(...SpecialCellLocation['SIGHASH_GROUP']), | ||
CODE_HASH: generateGenesisScriptConfig(...SpecialCellLocation['SIGHASH']).CODE_HASH, | ||
DEP_TYPE: 'depGroup', | ||
SHORT_ID: 0, |
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.
What does SHORT_ID
mean here?
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.
SHORT_ID is required for lumos to parse short address
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.
SHORT_ID is required for lumos to parse short address
I think it can be removed if it's not mandatory in lumos because short address is deprecated, and we should guide users not to use it.
1d45bd4
to
7122237
Compare
7122237
to
2b46199
Compare
Codecov Report
@@ Coverage Diff @@
## develop #242 +/- ##
==========================================
Coverage ? 68.59%
==========================================
Files ? 84
Lines ? 2305
Branches ? 515
==========================================
Hits ? 1581
Misses ? 657
Partials ? 67 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
feat: kuai contract support multisig deploy
I see feat: kuai contract support multisig deploy has been merged, and an example of 0/1 multisig tx was demonstrated in that PR. My question is how to share the partially signed transaction via And the example could be appended in this PR because the PR of feat: kuai contract support multisig deploy will be inconspicuous when it's one commit of this PR |
This feature is required by other features(deploy contract on devnet, complete workflow of building a dapp) so let's merge it first, other details could be added later |
deploy contract workflow:
kuai contract new --name [contract-name]
kuai contract build --release
kuai contract deploy --name [contract-name] --from [0x.....] --signer ckb-cli
Tips: How to deploy contract to other network
kuai contract deploy --name [contract-name] --from [0x.....] --network [networkname]
.env
fileTips: add more network config
Kuai currently has three default settings for networks,
testnet
/mainnet
/devnet
, and users can also add custom networkkuai.config.js
of project and add network tornetworks
field