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

refactor 2 #3

Closed
xhliu opened this issue Sep 18, 2023 · 28 comments
Closed

refactor 2 #3

xhliu opened this issue Sep 18, 2023 · 28 comments
Assignees
Labels
enhancement New feature or request

Comments

@xhliu
Copy link
Contributor

xhliu commented Sep 18, 2023

setConstructor -> init

this.setConstructor(counter)

@xhliu xhliu added the enhancement New feature or request label Sep 18, 2023
@xhliu
Copy link
Contributor Author

xhliu commented Sep 18, 2023

contentType: 'text/plain',

This is not raw hex, just a string

/** contentType in raw hex */

@xhliu
Copy link
Contributor Author

xhliu commented Sep 18, 2023

https://github.com/sCrypt-Inc/scrypt-ord/blob/09dbe2ad0d36f240055e8729027b97c4305b2922/tests/specs/hashPuzzle.spec.ts#L17C18-L17C24
const tick = toByteString('DOGE', true),

lim = max / 10
amt = lim

https://github.com/sCrypt-Inc/scrypt-ord/blob/master/tests/specs/hashPuzzle.spec.ts#L19-L21

move all to before()

const hashPuzzle = new HashPuzzle(
toByteString(tick, true),
max,
lim,
sha256(message1)
)
await hashPuzzle.connect(getDefaultSigner())
await hashPuzzle.deployToken()
await hashPuzzle.mint(amt)

@xhliu
Copy link
Contributor Author

xhliu commented Sep 18, 2023

Looks like first output is token change? If yes, let's move it to the last, as sat change.

const { tx } = await next0.methods.unlock(message1)

Also, add comments throughout to explain, e.g.,

  1. what are nexts?
  2. Why is this a burn?
  3. What are outputs when callContract?

@xhliu
Copy link
Contributor Author

xhliu commented Sep 18, 2023

@xhliu
Copy link
Contributor Author

xhliu commented Sep 18, 2023

Change to any amount other than 1, which is confusing to 1sat.

BSV20V1.buildTransferOutput(address, this.tick, 1n) +

@xhliu
Copy link
Contributor Author

xhliu commented Sep 18, 2023

Rename build1SatStateOutput -> buildStateOutputFT & buildStateOutputNFT. 1Sat is too general here.
https://github.com/sCrypt-Inc/scrypt-ord/blob/09dbe2ad0d36f240055e8729027b97c4305b2922/tests/contracts/ftCounter.ts#L28C31-L28C31

@xhliu
Copy link
Contributor Author

xhliu commented Sep 18, 2023

Rename changeAmt -> changeTokenAmt
https://github.com/sCrypt-Inc/scrypt-ord/blob/09dbe2ad0d36f240055e8729027b97c4305b2922/tests/specs/ftCounter.spec.ts#L47C19-L47C28

@xhliu
Copy link
Contributor Author

xhliu commented Sep 18, 2023

Add comments for all files under https://github.com/sCrypt-Inc/scrypt-ord/tree/master/src/contracts, at least for all top-level functions.

@zhfnjust
Copy link
Collaborator

Looks like first output is token change? If yes, let's move it to the last, as sat change.

const { tx } = await next0.methods.unlock(message1)

Also, add comments throughout to explain, e.g.,

  1. what are nexts?
  2. Why is this a burn?
  3. What are outputs when callContract?
  1. update transfer() returns , no more nexts, just
Promise<{
        /** The method calling tx */
        tx: bsv.Transaction;
        changeInstance: BSV20V1 | null
        receivers: Array<BSV20V1>
    }>
  1. add a burn method, so don't need to tell why this is a burn.
  2. first output is a instance holds change token amt, the secound output is the first receiver, the third output is the secound receiver. last output is change output.

@xhliu
Copy link
Contributor Author

xhliu commented Sep 20, 2023

hash160 -> pubKey2Addr

assert(hash160(pubkey) == this.addr, 'public key hashes are not equal')

@xhliu
Copy link
Contributor Author

xhliu commented Sep 20, 2023

const tick = toByteString('DOGE', true)
https://github.com/sCrypt-Inc/scrypt-ord/blob/master/tests/specs/hashPuzzle.spec.ts#L12C5-L12C24

@xhliu
Copy link
Contributor Author

xhliu commented Sep 20, 2023

Let's make the API as close to the original sCrypt (without ordinals) as possible. It is still sCrypt but extended to support ordinals.

const recipients =                     [
                        {
                            instance: new HashPuzzle(
                                toByteString(tick, true),
                                max,
                                lim,
                                sha256(message2)
                            ),
                            amt: 10n,
                        },
                    ]
const { tx, atInputIndex } = await hashPuzzle.methods.unlock(message1, { transfer: recipients, tokenChangeAddress: tokenChangeAddr} as OrdMethodCallOptions)

OrdMethodCallOptions extends MethodCallOptions. tokenChangeAddress behaves same as changeAddress: if not set, token change goes to signer.getDefaultAddress().

const { tx, tokenChangeP2PKH, receivers } =
await hashPuzzle.transfer(
[
{
instance: new HashPuzzle(
toByteString(tick, true),
max,
lim,
sha256(message2)
),
amt: 10n,
},
],
'unlock',
message1
)

@xhliu
Copy link
Contributor Author

xhliu commented Sep 21, 2023

Make

it('transfer to an other hashPuzzle and withdraw.', async () => {

similar to
https://github.com/sCrypt-Inc/boilerplate/blob/5670d548bbc0d7502f037bb0ae20862ca58a89f9/tests/counter.test.ts#L11

Call/transfer multiple times in a chain of txs.

@xhliu
Copy link
Contributor Author

xhliu commented Sep 21, 2023

No need to specially handle withdraw, it's just another transfer to an instance. I suggest removing it.

const withdraw = async () => {
const address = await hashPuzzle.signer.getDefaultAddress()
const recipients = [
{
instance: OrdP2PKH.fromAddress(address),
amt: hashPuzzle.getAmt(),
},
]
const { tx } = await hashPuzzle.methods.unlock(
toByteString(`hello, sCrypt!:3`, true),
{
transfer: recipients,
}
)
console.log('withdraw tx: ', tx.id)
}
await expect(withdraw()).not.rejected

@xhliu
Copy link
Contributor Author

xhliu commented Sep 21, 2023

Does it support inscribing NFT to a p2pkh? https://github.com/sCrypt-Inc/scrypt-ord/blob/master/src/contracts/ordP2PKH.ts#L25

@xhliu
Copy link
Contributor Author

xhliu commented Sep 21, 2023

Move this out of contracts folder to, say, src/utils.ts.

@xhliu
Copy link
Contributor Author

xhliu commented Sep 21, 2023

ordP2PKH = new OrdP2PKH(address)

ordP2PKH = OrdP2PKH.fromAddress(address)

Same everywhere

instance: OrdP2PKH.fromAddress(address),

@xhliu
Copy link
Contributor Author

xhliu commented Sep 21, 2023

No need to specially handle p2pkh. Can we use the generalized transfer, or remove the whole test file?
https://github.com/sCrypt-Inc/scrypt-ord/blob/62d5ee7a9e7662f7ebb5aa60b359aab402566f25/tests/specs/ordP2PKH.spec.ts

E.g., this can be just like any other transfer: ordP2PKH.methods.unlock

const { tx, nexts } = await ordP2PKH.transferBsv20(

@xhliu
Copy link
Contributor Author

xhliu commented Sep 21, 2023

Just reuse fromTx() from contractID https://docs.scrypt.io/how-to-deploy-and-call-a-contract/call-deployed#interact. Again, make API as close as before.

hashPuzzle = HashPuzzle.fromUTXO(

Do NOT need tx details to call, such as

script: '0063036f726451126170706c69636174696f6e2f6273762d323000367b2270223a226273762d3230222c226f70223a227472616e73666572222c227469636b223a224c554e43222c22616d74223a2236227d6800000000044c554e4304406f400104406f400120eb2c9ad39d81bbe2acbec6f42859d94f86226186838560f3a698d9047f49ce72615379577a75567a567a567a567a567a567a51587a75577a577a577a577a577a577a577a0079567a75557a557a557a557a557a757575756151795579a88777777777776a00010200000000',

@xhliu
Copy link
Contributor Author

xhliu commented Sep 21, 2023

This is gonna cover #5?

@xhliu
Copy link
Contributor Author

xhliu commented Sep 21, 2023

@xhliu
Copy link
Contributor Author

xhliu commented Sep 22, 2023

Let's remove all places where p2pkh is treated differently from a contract. It is just ONE contract.
Like

static send2Contract(

@zhfnjust
Copy link
Collaborator

Does it support inscribing NFT to a p2pkh? https://github.com/sCrypt-Inc/scrypt-ord/blob/master/src/contracts/ordP2PKH.ts#L25

yes

@zhfnjust
Copy link
Collaborator

Do NOT need tx details to call, such as

yes

@xhliu
Copy link
Contributor Author

xhliu commented Sep 22, 2023

Use a CONST variable for this magic number. Same everywhere.

const P2pkhScriptLen = 50

? ls.toHex().slice(50)

const nopScript = bsv.Script.fromHex(utxo.script.slice(50))

@xhliu
Copy link
Contributor Author

xhliu commented Sep 22, 2023

readonly and comment saying it's dummy. @zhfnjust

isBSV20V1: boolean

@xhliu
Copy link
Contributor Author

xhliu commented Sep 25, 2023

@zhfnjust close this after all is fixed.

@zhfnjust
Copy link
Collaborator

zhfnjust commented Oct 7, 2023

Add comments for all files under https://github.com/sCrypt-Inc/scrypt-ord/tree/master/src/contracts, at least for all top-level functions.

#37

@zhfnjust zhfnjust closed this as completed Oct 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants