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

Enable payload continue #8

Merged

Conversation

biryukovmaxim
Copy link

No description provided.

biryukovmaxim and others added 10 commits November 12, 2024 12:10
…ard Fork Support (kaspanet#487)

* implement new opcodes

* example of mutual tx

* add docs describing scenario

* introduce feature gate for new features

* introduce hf feature that enables txscript hf feature

* style: fmt and clippy fix

* implement new opcodes

* example of mutual tx

* add docs describing scenario

* introduce feature gate for new features

* style: fmt and clippy fix

* remove unused feature

* fmt

* make opcode invalid in case of feature disabled

* feature gate test

* change test set based on feature
add ci cd test

* rename InputSPK -> InputSpk

* enable kip10 opcodes based on daa_score in runtime

* use dummy kip10 activation daa score in params

* use dummy kip10 activation daa score in params

* suppress clippy lint

* add example with shared key

* fix clippy

* remove useless check from example

* add one-time borrowing example

* Implement one-time and two-times threshold borrowing scenarios

- Add threshold_scenario_limited_one_time function
- Add threshold_scenario_limited_2_times function
- Create generate_limited_time_script for reusable script generation
- Implement nested script structure for two-times borrowing
- Update documentation for both scenarios
- Add tests for owner spending, borrowing, and invalid attempts in both cases
- Ensure consistent error handling and logging across scenarios
- Refactor to use more generic script generation approach

* fix: fix incorrect sig-op count

* correct error description

* style: fmt

* pass kip-10 flag in constructor params

* remove borrow scenario from tests.
run tests against both kip1- enabled/disabled engine

* introduce method that converts spk to bytes.
add tests covering new opcodes

* return comment describing where invalid opcodes starts from.
add comments describing why 2 files are used.

* fix wring error messages

* support introspection by index

* test input spk

* test output spk

* tests refactor

* support 8-byte arithmetics

* Standartize fork activation logic (kaspanet#588)

* Use ForkActivation for all fork activations

* Avoid using negation in some ifs

* Add is_within_range_from_activation

* Move 'is always' check inside is_within_range_from_activation

* lints

* Refactoring for cleaner pruning proof module (kaspanet#589)

* Cleanup manual block level calc

There were two areas in pruning proof mod that
manually calculated block level.

This replaces those with a call to calc_block_level

* Refactor pruning proof build functions

* Refactor apply pruning proof functions

* Refactor validate pruning functions

* Add comments for clarity

* only enable 8 byte arithmetics for kip10

* use i64 value in 9-byte tests

* fix tests covering kip10 and i64 deserialization

* fix test according to 8-byte math

* finish test covering kip10 opcodes: input/output/amount/spk

* fix kip10 examples

* rename test

* feat: add input index op

* feat: add input/outpiut opcodes

* reseve opcodes
reorder kip10 opcodes.
reflect script tests

* fix example

* introspection opcodes are reserverd, not disables

* use ForkActivation type

* cicd: run kip-10 example

* move spk encoding to txscript module

* rework bound check ot input/output index

* fix tests by importing spkencoding trait

* replace todo in descripotions of over[under]flow errors

* reorder new opcodes, reserve script sig opcode, remove txid

* fix bitcoin script tests

* add simple opcode tests

* rename id(which represents input index) to idx

* fix comments

* add input spk tests

* refactor test cases

* refactor(txscript): Enforce input index invariant via assertion

Change TxScriptEngine::from_transaction_input to assert valid input index
instead of returning Result. This better reflects that an invalid index is a
caller's (transaction validation) error rather than a script engine error,
since the input must be part of the transaction being validated.

An invalid index signifies a mismatch between the transaction and the input being
validated - this is a programming error in the transaction validator layer, not
a script engine concern. The script engine should be able to assume it receives
valid inputs from its caller.

The change simplifies error handling by enforcing this invariant early,
while maintaining identical behavior for valid inputs. The function is
now documented to panic on malformed inputs.

This is a breaking change for code that previously handled
InvalidIndex errors, though such handling was likely incorrect
as it indicated an inconsistency in transaction validation.

* refactor error types to contain correct info

* rename id to idx

* rename opcode

* make construction of TxScriptEngine from transaction input infallible

* style: format combinators chain

* add integration test covering activation of kip10

* rename kip10_activation_daa_score to kip10_activation

* Update crypto/txscript/src/lib.rs

refactor vector filling

* rework assert

* verify that block is disqualified in case of it has tx which requires
block that contains the tx with kip10 opcode is accepted after daa score has being reached

* revert changer to infallible api

* add doc comments

* Update crypto/txscript/src/opcodes/mod.rs

Fallible conversion of output amount (usize -> i64)

* Update crypto/txscript/src/opcodes/mod.rs

Fallible conversion of input amount (usize -> i64)

* add required import

* refactor: SigHashReusedValuesUnsync doesnt neet to be mutable

* fix test description

* rework example

* 9 byte integers must fail to serialize

* add todo

* rewrite todo

* remove redundant code

* remove redundant mut in example

* remove redundant mut in example

* remove redundant mut in example

* cicd: apply lint to examples

---------

Co-authored-by: Ori Newman <orinewman1@gmail.com>
* Some simplification to script number types

* Add TODO

* Address review comments
* feat: add signMessageWithoutRand method for kaspa wasm

* enhance: sign message api

* fix: unit test fail

* chore: update noAuxRand of ISignMessage

* chore: add sign message demo for noAuxRand
Refactored `mock_tx` to `mock_tx_with_payload` to support custom payload sizes. Introduced new benchmark function `benchmark_check_scripts_with_payload` to test performance with varying payload sizes. Commented out the old benchmark function to focus on payload-based tests.
Added benchmarks to evaluate script checking performance with varying payload sizes and input counts. This helps in understanding the impact of transaction payload size on validation and the relationship between input count and payload processing overhead.
This commit introduces a new test case to verify that transaction IDs and hashes change with payload modifications. Additionally, code readability and consistency are improved by refactoring multi-line expressions into single lines where appropriate.
# Conflicts:
#	consensus/src/consensus/services.rs
#	consensus/src/processes/transaction_validator/mod.rs
#	testing/integration/src/consensus_integration_tests.rs
This commit introduces a new integration test to validate the enforcement of payload activation rules at a specified DAA score. The test ensures that transactions with large payloads are rejected before activation and accepted afterward, maintaining consensus integrity.
config = Criterion::default().with_output_color(true).measurement_time(std::time::Duration::new(20, 0));
targets = benchmark_check_scripts
targets = /*benchmark_check_scripts,*/ benchmark_check_scripts_with_payload
Copy link
Owner

Choose a reason for hiding this comment

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

Don't leave redundant comments

Copy link
Author

Choose a reason for hiding this comment

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

It's already been removed

@@ -157,6 +157,13 @@ mod tests {
expected_hash: "31da267d5c34f0740c77b8c9ebde0845a01179ec68074578227b804bac306361",
});

// Test #8, same as 7 but with a non-zero payload. The test checks id and hash are affected by payload change
Copy link
Owner

Choose a reason for hiding this comment

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

Please add similar test to test_signature_hash

Copy link
Author

Choose a reason for hiding this comment

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

commit 03a8ca8

@@ -1903,3 +1904,114 @@ async fn payload_test() {

consensus.shutdown(wait_handles);
}

#[tokio::test]
async fn run_payload_activation_test() {
Copy link
Owner

Choose a reason for hiding this comment

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

There's no need for run_ in test names

Copy link
Author

Choose a reason for hiding this comment

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

commit 2df7765

Copy link
Owner

@someone235 someone235 left a comment

Choose a reason for hiding this comment

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

Also, remove from consensus/core/src/hashing/sighash.rs comment saying // TODO: Right now this branch will never be executed, since payload is disabled

@biryukovmaxim
Copy link
Author

Also, remove from consensus/core/src/hashing/sighash.rs comment saying // TODO: Right now this branch will never be executed, since payload is disabled

It's already been removed

@someone235 someone235 merged commit e9711ac into someone235:enable-payload Nov 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants