Skip to content

bug: fix payment check conflict and flag impact for type hash#32

Merged
ashuralyk merged 8 commits into
masterfrom
bug/payment_and_type_hash
Jan 11, 2024
Merged

bug: fix payment check conflict and flag impact for type hash#32
ashuralyk merged 8 commits into
masterfrom
bug/payment_and_type_hash

Conversation

@ashuralyk
Copy link
Copy Markdown
Contributor

@ashuralyk ashuralyk commented Jan 8, 2024

Description

payment function needs to pin a flag in script.args filed, which can impact the calculation of script hash, however, if this payment flag changed in transfer operation, we cannot just use GroupInput and GroupOutput to identify the related scripts both in Input and Output field, so this change of payment should be described as Burn/Mint in one transaction.

when we use payment flag to check the user's payment is wether enough, since different scripts in Mutant or Proxy cell act independently, and these scripts use the same logic to run a payment check process, so, for example, if a same owner of two Mutant cells with different mutant_id disappeared in CellDep field at the same time, one payment in transaction will be duplicated in use to meet this two Mutant scripts' requirement.

Tasks list

@ashuralyk ashuralyk self-assigned this Jan 8, 2024
Flouse

This comment was marked as resolved.

@ashuralyk
Copy link
Copy Markdown
Contributor Author

ashuralyk commented Jan 9, 2024

Currently, Mint/Creation operation can't set minimal_payment_args either?

the change of minimal_payment_args can influent the calculation of type id, known as proxy id here, so different proxy id both in Input and Output cannot recognize as transfer operation, it's just burn and mint instead

@Flouse

@ashuralyk ashuralyk force-pushed the bug/payment_and_type_hash branch from 89153b3 to 80cbd95 Compare January 9, 2024 13:13
@ashuralyk
Copy link
Copy Markdown
Contributor Author

ashuralyk commented Jan 10, 2024

Mint/Creation operation can set a ClusterProxy's minimal_payment_args after the type_id

what does this mean? payment args is written in script args, which causes the change of type_id and cannot be altered
@Flouse

@ashuralyk ashuralyk force-pushed the bug/payment_and_type_hash branch from 55f496b to dcdaf54 Compare January 10, 2024 04:00
@Flouse
Copy link
Copy Markdown
Contributor

Flouse commented Jan 10, 2024

what does this mean? payment args is written in script args, which causes the change of type_id and cannot be altered

I have no questions now.
This PR is approved 13 hours ago.

@ashuralyk ashuralyk force-pushed the bug/payment_and_type_hash branch 8 times, most recently from 193f215 to c745173 Compare January 10, 2024 13:36
@ashuralyk ashuralyk force-pushed the bug/payment_and_type_hash branch from c745173 to 33514c9 Compare January 10, 2024 13:38
@ashuralyk ashuralyk requested a review from Flouse January 11, 2024 02:34
@ashuralyk
Copy link
Copy Markdown
Contributor Author

ashuralyk commented Jan 11, 2024

@Flouse @ShookLyngs I committed some changes, please check again, I closed the other two issues that hasn't been closed

@ashuralyk ashuralyk force-pushed the bug/payment_and_type_hash branch from cf5fd9e to 01878bf Compare January 11, 2024 02:41
@ashuralyk ashuralyk requested a review from roughpandaz January 11, 2024 02:51

- name: Prepare spore-devenv (contracts and stuff)
working-directory: spore-devenv
run: bash prepare.sh -b $GITHUB_HEAD_REF
Copy link
Copy Markdown
Contributor

@Flouse Flouse Jan 11, 2024

Choose a reason for hiding this comment

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

Suggested change
run: bash prepare.sh -b $GITHUB_HEAD_REF
run: bash prepare.sh -b $GITHUB_REF

GITHUB_HEAD_REF is the head ref or source branch of the pull request in a workflow run. This property is only set when the event that triggers a workflow run is either pull_request or pull_request_target.

Suggestions

  • Add a push event for this workflow, since it only takes 10 minutes.
    Running devnet test for every pushed commit is acceptable.

  • use GITHUB_REF instead of GITHUB_HEAD_REF

    GITHUB_REF: The fully-formed ref of the branch or tag that triggered the workflow run. For workflows triggered by push, this is the branch or tag ref that was pushed. For workflows triggered by pull_request, this is the pull request merge branch. For workflows triggered by release, this is the release tag created.
    See https://docs.github.com/en/actions/learn-github-actions/variables

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

we used $GITHUB_REF but did not get to our expectation, and for now, $GITHUB_HEAD_REF works fine for us, so it can stay as it is now

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What do you mean it did not get to our expectation?
Is there a GitHub action that contains the error?

Comment thread .github/workflows/devnet.yaml
@Flouse
Copy link
Copy Markdown
Contributor

Flouse commented Jan 11, 2024

bug: fix remained bugs
I closed the other two issues that hasn't been closed

Next time, it would be nice if you create new PR for each issue.
You know, one PR for one issue.
This allows for better tracking, review, and management of changes.

Comment thread lib/utils/src/mime.rs
));
)
.map_err(|_| "mutant verify_param")
.unwrap());
Copy link
Copy Markdown
Contributor

@Flouse Flouse Jan 11, 2024

Choose a reason for hiding this comment

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

These changes may conflict with 1e100f6 in your another PR. You will need to resolve the conflicts later which may take more time.

Again, it is better to solve one small issue in one PR.
For now, it LGTM.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes, next step is to solve conflict, the time is ok

Comment thread lib/utils/src/mime.rs Outdated
Co-authored-by: Flouse <1297478+Flouse@users.noreply.github.com>
Comment thread contracts/spore/src/entry.rs
Co-authored-by: Flouse <1297478+Flouse@users.noreply.github.com>
@ashuralyk ashuralyk merged commit 3d84f66 into master Jan 11, 2024
@ashuralyk ashuralyk deleted the bug/payment_and_type_hash branch January 11, 2024 07:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants