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

feature/pos-syncing-and-steady-state #875

Merged

Conversation

tholonious
Copy link
Contributor

No description provided.

tholonious and others added 30 commits November 1, 2023 10:28
…file (#763)

* Create basic skeleton for PoS message handling and syncing

* Simplify interface

* Create new struct

* Create constrtuctor for ConsensusMessageHandler

* rename ConsensusMessageHandler to ConsensusController
* Instantiated FastHotStuffEventLoop within ConsensusController

* Address Nina's comments
* Handle And Broadcast Local Timeout Signal

* Address Nina's feedback

* Fix signature verification
* Handle And Broadcast Local Vote Signal

* Fix signature verification
…message_construction_for_broadcast

Clean up vote and timeout message construction for broadcast
* Add ConsensusController Utility to Fetch Validator Set For Blocks

Cleanup

Address Nina's comments

* Better comments
* Handle Block Construction Signal and Broadcast

* Address Nina's comments
* Handle Timeout Block Construction Signal and Broadcast

* Address Nina's comments
* Remove Stubbed Out Functions For Node Syncing

* Add build flag
* Sign Random Seed Hash in PoS Block Proposal

* Address Nina's nits
AeonSw4n and others added 3 commits February 2, 2024 12:06
… TxnConnects condition (#1009)

* PoS Change ForkHeight Checks to Use DeSoParams; Simplify BlockView TxnConnects condition

* nit
)

* Clarify Block Height Condition For Starting FastHotStuffConsensus

* Clean up comment
@diamondhands0
Copy link
Member

In block_view.go there is a line effectiveFeeU256 := failingTransactionRate.Mul(failingTransactionRate, failingTransactionFee). Change it to effectiveFeeU256 := uint256.NewInt().Mul(failingTransactionRate, failingTransactionFee)

Always use NewInt when assigning to a new variable.

@diamondhands0
Copy link
Member

Also in block_view.go:3928, I think you could overflow after the multiplication.

	effectiveFeeU256 := failingTransactionRate.Mul(failingTransactionRate, failingTransactionFee)
        // Add a check for overflow here. 
	effectiveFeeU256.Div(effectiveFeeU256, basisPointsAsUint256)

I think for multiplication, you check that the resulting value is bigger than each of the two multiplied values.

@diamondhands0
Copy link
Member

diamondhands0 commented Feb 4, 2024

Also in block_view.go:_connectFailingTransaction on line 3938, I think you're comparing a raw txn fee to a fee rate:

	if effectiveFee < gp.MinimumNetworkFeeNanosPerKB {
		effectiveFee = gp.MinimumNetworkFeeNanosPerKB
	}

To fix this, I think you need to multiply by 1k and divide by the size of the txn, and maybe add one in the comparison to cover truncation. Then you need to set effectiveFee = minimum * 1000 / txnSize.

Also add some tests to check this.

AeonSw4n and others added 2 commits February 7, 2024 10:00
* PoS SHA3 Random Seed

* Fix Test
* Fix Txn Fee Validation in UtxoView._connectFailingTransaction

* Cleanup
@tholonious tholonious merged commit 78ce1f5 into feature/proof-of-stake Feb 7, 2024
3 checks passed
@tholonious tholonious deleted the feature/pos-syncing-and-steady-state branch February 7, 2024 20:39
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.

4 participants